From 5855a14935abaa318b66dbd0fc1b9a14c39ad7e8 Mon Sep 17 00:00:00 2001 From: Tom Pallister Date: Sat, 9 Dec 2017 14:41:35 +0000 Subject: [PATCH] Feature/more validation (#174) * added message assertion for validation test * another message assertion * more validation tests --- .../FileConfigurationFluentValidator.cs | 31 ++- .../Validator/ReRouteFluentValidator.cs | 25 +- .../ConfigurationFluentValidationTests.cs | 234 +++++++++++++++++- 3 files changed, 272 insertions(+), 18 deletions(-) diff --git a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs index 5cc4967b..08ab574a 100644 --- a/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs +++ b/src/Ocelot/Configuration/Validator/FileConfigurationFluentValidator.cs @@ -15,35 +15,50 @@ namespace Ocelot.Configuration.Validator { RuleFor(configuration => configuration.ReRoutes) .SetCollectionValidator(new ReRouteFluentValidator(authenticationSchemeProvider)); + RuleForEach(configuration => configuration.ReRoutes) .Must((config, reRoute) => IsNotDuplicateIn(reRoute, config.ReRoutes)) - .WithMessage((config, reRoute) => $"duplicate downstreampath {reRoute.UpstreamPathTemplate}"); + .WithMessage((config, reRoute) => $"{nameof(reRoute)} {reRoute.UpstreamPathTemplate} has duplicate"); } public async Task> IsValid(FileConfiguration configuration) { var validateResult = await ValidateAsync(configuration); + if (validateResult.IsValid) { return new OkResponse(new ConfigurationValidationResult(false)); } + var errors = validateResult.Errors.Select(failure => new FileValidationFailedError(failure.ErrorMessage)); + var result = new ConfigurationValidationResult(true, errors.Cast().ToList()); + return new OkResponse(result); } - private static bool IsNotDuplicateIn(FileReRoute reRoute, List routes) + private static bool IsNotDuplicateIn(FileReRoute reRoute, List reRoutes) { - var reRoutesWithUpstreamPathTemplate = routes.Where(r => r.UpstreamPathTemplate == reRoute.UpstreamPathTemplate).ToList(); - var hasEmptyListToAllowAllHttpVerbs = reRoutesWithUpstreamPathTemplate.Any(x => x.UpstreamHttpMethod.Count == 0); - var hasDuplicateEmptyListToAllowAllHttpVerbs = reRoutesWithUpstreamPathTemplate.Count(x => x.UpstreamHttpMethod.Count == 0) > 1; + var matchingReRoutes = reRoutes.Where(r => r.UpstreamPathTemplate == reRoute.UpstreamPathTemplate).ToList(); - var hasSpecificHttpVerbs = reRoutesWithUpstreamPathTemplate.Any(x => x.UpstreamHttpMethod.Count != 0); - var hasDuplicateSpecificHttpVerbs = reRoutesWithUpstreamPathTemplate.SelectMany(x => x.UpstreamHttpMethod).GroupBy(x => x.ToLower()).SelectMany(x => x.Skip(1)).Any(); - if (hasDuplicateEmptyListToAllowAllHttpVerbs || hasDuplicateSpecificHttpVerbs || (hasEmptyListToAllowAllHttpVerbs && hasSpecificHttpVerbs)) + if(matchingReRoutes.Count == 1) + { + return true; + } + + var allowAllVerbs = matchingReRoutes.Any(x => x.UpstreamHttpMethod.Count == 0); + + var duplicateAllowAllVerbs = matchingReRoutes.Count(x => x.UpstreamHttpMethod.Count == 0) > 1; + + var specificVerbs = matchingReRoutes.Any(x => x.UpstreamHttpMethod.Count != 0); + + var duplicateSpecificVerbs = matchingReRoutes.SelectMany(x => x.UpstreamHttpMethod).GroupBy(x => x.ToLower()).SelectMany(x => x.Skip(1)).Any(); + + if (duplicateAllowAllVerbs || duplicateSpecificVerbs || (allowAllVerbs && specificVerbs)) { return false; } + return true; } } diff --git a/src/Ocelot/Configuration/Validator/ReRouteFluentValidator.cs b/src/Ocelot/Configuration/Validator/ReRouteFluentValidator.cs index 915be18b..b386890f 100644 --- a/src/Ocelot/Configuration/Validator/ReRouteFluentValidator.cs +++ b/src/Ocelot/Configuration/Validator/ReRouteFluentValidator.cs @@ -17,19 +17,35 @@ namespace Ocelot.Configuration.Validator RuleFor(reRoute => reRoute.DownstreamPathTemplate) .Must(path => path.StartsWith("/")) - .WithMessage("downstream path {PropertyValue} doesnt start with forward slash"); + .WithMessage("{PropertyName} {PropertyValue} doesnt start with forward slash"); + RuleFor(reRoute => reRoute.UpstreamPathTemplate) .Must(path => path.StartsWith("/")) - .WithMessage("upstream path {PropertyValue} doesnt start with forward slash"); + .WithMessage("{PropertyName} {PropertyValue} doesnt start with forward slash"); + RuleFor(reRoute => reRoute.DownstreamPathTemplate) .Must(path => !path.Contains("https://") && !path.Contains("http://")) - .WithMessage("downstream path {PropertyValue} contains scheme"); + .WithMessage("{PropertyName} {PropertyValue} contains scheme"); + + RuleFor(reRoute => reRoute.UpstreamPathTemplate) + .Must(path => !path.Contains("https://") && !path.Contains("http://")) + .WithMessage("{PropertyName} {PropertyValue} contains scheme"); + RuleFor(reRoute => reRoute.RateLimitOptions) .Must(IsValidPeriod) - .WithMessage("rate limit period {PropertyValue} not contains (s,m,h,d)"); + .WithMessage("RateLimitOptions.Period does not contains (s,m,h,d)"); + RuleFor(reRoute => reRoute.AuthenticationOptions) .MustAsync(IsSupportedAuthenticationProviders) .WithMessage("{PropertyValue} is unsupported authentication provider"); + + When(reRoute => reRoute.UseServiceDiscovery, () => { + RuleFor(r => r.ServiceName).NotEmpty().WithMessage("ServiceName cannot be empty or null when using service discovery or Ocelot cannot look up your service!"); + }); + + When(reRoute => !reRoute.UseServiceDiscovery, () => { + RuleFor(r => r.DownstreamHost).NotEmpty().WithMessage("When not using service discover DownstreamHost must be set or Ocelot cannot find your service!"); + }); } private async Task IsSupportedAuthenticationProviders(FileAuthenticationOptions authenticationOptions, CancellationToken cancellationToken) @@ -39,6 +55,7 @@ namespace Ocelot.Configuration.Validator return true; } var schemes = await _authenticationSchemeProvider.GetAllSchemesAsync(); + var supportedSchemes = schemes.Select(scheme => scheme.Name).ToList(); return supportedSchemes.Contains(authenticationOptions.AuthenticationProviderKey); diff --git a/test/Ocelot.UnitTests/Configuration/ConfigurationFluentValidationTests.cs b/test/Ocelot.UnitTests/Configuration/ConfigurationFluentValidationTests.cs index 6a1bac80..90475a27 100644 --- a/test/Ocelot.UnitTests/Configuration/ConfigurationFluentValidationTests.cs +++ b/test/Ocelot.UnitTests/Configuration/ConfigurationFluentValidationTests.cs @@ -29,7 +29,7 @@ namespace Ocelot.UnitTests.Configuration } [Fact] - public void configuration_is_invalid_if_scheme_in_downstream_template() + public void configuration_is_invalid_if_scheme_in_downstream_or_upstream_template() { this.Given(x => x.GivenAConfiguration(new FileConfiguration { @@ -45,6 +45,10 @@ namespace Ocelot.UnitTests.Configuration .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) .Then(x => x.ThenTheErrorIs()) + .And(x => x.ThenTheErrorMessageAtPositionIs(0, "Downstream Path Template http://www.bbc.co.uk/api/products/{productId} doesnt start with forward slash")) + .And(x => x.ThenTheErrorMessageAtPositionIs(1, "Upstream Path Template http://asdf.com doesnt start with forward slash")) + .And(x => x.ThenTheErrorMessageAtPositionIs(2, "Downstream Path Template http://www.bbc.co.uk/api/products/{productId} contains scheme")) + .And(x => x.ThenTheErrorMessageAtPositionIs(3, "Upstream Path Template http://asdf.com contains scheme")) .BDDfy(); } @@ -58,7 +62,8 @@ namespace Ocelot.UnitTests.Configuration new FileReRoute { DownstreamPathTemplate = "/api/products/", - UpstreamPathTemplate = "/asdf/" + UpstreamPathTemplate = "/asdf/", + DownstreamHost = "bbc.co.uk" } } })) @@ -83,6 +88,7 @@ namespace Ocelot.UnitTests.Configuration })) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) + .And(x => x.ThenTheErrorMessageAtPositionIs(0, "Downstream Path Template api/products/ doesnt start with forward slash")) .BDDfy(); } @@ -102,6 +108,7 @@ namespace Ocelot.UnitTests.Configuration })) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) + .And(x => x.ThenTheErrorMessageAtPositionIs(0, "Upstream Path Template api/prod/ doesnt start with forward slash")) .BDDfy(); } @@ -116,6 +123,7 @@ namespace Ocelot.UnitTests.Configuration { DownstreamPathTemplate = "/api/products/", UpstreamPathTemplate = "/asdf/", + DownstreamHost = "bbc.co.uk", AuthenticationOptions = new FileAuthenticationOptions() { AuthenticationProviderKey = "Test" @@ -148,11 +156,12 @@ namespace Ocelot.UnitTests.Configuration })) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) + .And(x => x.ThenTheErrorMessageAtPositionIs(0, "AuthenticationProviderKey:Test,AllowedScopes:[] is unsupported authentication provider")) .BDDfy(); } [Fact] - public void configuration_is_not_valid_with_duplicate_reroutes() + public void configuration_is_not_valid_with_duplicate_reroutes_all_verbs() { this.Given(x => x.GivenAConfiguration(new FileConfiguration { @@ -161,17 +170,225 @@ namespace Ocelot.UnitTests.Configuration new FileReRoute { DownstreamPathTemplate = "/api/products/", - UpstreamPathTemplate = "/asdf/" + UpstreamPathTemplate = "/asdf/", + DownstreamHost = "bb.co.uk" }, new FileReRoute { - DownstreamPathTemplate = "http://www.bbc.co.uk", - UpstreamPathTemplate = "/asdf/" + DownstreamPathTemplate = "/www/test/", + UpstreamPathTemplate = "/asdf/", + DownstreamHost = "bb.co.uk" } } })) .When(x => x.WhenIValidateTheConfiguration()) .Then(x => x.ThenTheResultIsNotValid()) + .And(x => x.ThenTheErrorMessageAtPositionIs(0, "reRoute /asdf/ has duplicate")) + .BDDfy(); + } + + [Fact] + public void configuration_is_not_valid_with_duplicate_reroutes_specific_verbs() + { + this.Given(x => x.GivenAConfiguration(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/api/products/", + UpstreamPathTemplate = "/asdf/", + DownstreamHost = "bbc.co.uk", + UpstreamHttpMethod = new List {"Get"} + }, + new FileReRoute + { + DownstreamPathTemplate = "/www/test/", + UpstreamPathTemplate = "/asdf/", + DownstreamHost = "bbc.co.uk", + UpstreamHttpMethod = new List {"Get"} + } + } + })) + .When(x => x.WhenIValidateTheConfiguration()) + .Then(x => x.ThenTheResultIsNotValid()) + .And(x => x.ThenTheErrorMessageAtPositionIs(0, "reRoute /asdf/ has duplicate")) + .BDDfy(); + } + + [Fact] + public void configuration_is_valid_with_duplicate_reroutes_different_verbs() + { + this.Given(x => x.GivenAConfiguration(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/api/products/", + UpstreamPathTemplate = "/asdf/", + UpstreamHttpMethod = new List {"Get"}, + DownstreamHost = "bbc.co.uk", + }, + new FileReRoute + { + DownstreamPathTemplate = "/www/test/", + UpstreamPathTemplate = "/asdf/", + UpstreamHttpMethod = new List {"Post"}, + DownstreamHost = "bbc.co.uk", + } + } + })) + .When(x => x.WhenIValidateTheConfiguration()) + .Then(x => x.ThenTheResultIsValid()) + .BDDfy(); + } + + [Fact] + public void configuration_is_invalid_with_invalid_rate_limit_configuration() + { + this.Given(x => x.GivenAConfiguration(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/api/products/", + UpstreamPathTemplate = "/asdf/", + UpstreamHttpMethod = new List {"Get"}, + DownstreamHost = "bbc.co.uk", + RateLimitOptions = new FileRateLimitRule + { + Period = "1x", + EnableRateLimiting = true + } + } + } + })) + .When(x => x.WhenIValidateTheConfiguration()) + .Then(x => x.ThenTheResultIsNotValid()) + .And(x => x.ThenTheErrorMessageAtPositionIs(0, "RateLimitOptions.Period does not contains (s,m,h,d)")) + .BDDfy(); + } + + [Fact] + public void configuration_is_valid_with_valid_rate_limit_configuration() + { + this.Given(x => x.GivenAConfiguration(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/api/products/", + UpstreamPathTemplate = "/asdf/", + UpstreamHttpMethod = new List {"Get"}, + DownstreamHost = "bbc.co.uk", + RateLimitOptions = new FileRateLimitRule + { + Period = "1d", + EnableRateLimiting = true + } + } + } + })) + .When(x => x.WhenIValidateTheConfiguration()) + .Then(x => x.ThenTheResultIsValid()) + .BDDfy(); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + public void configuration_is_invalid_with_using_service_discovery_and_no_service_name(string serviceName) + { + this.Given(x => x.GivenAConfiguration(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/api/products/", + UpstreamPathTemplate = "/asdf/", + UpstreamHttpMethod = new List {"Get"}, + UseServiceDiscovery = true, + ServiceName = serviceName + } + } + })) + .When(x => x.WhenIValidateTheConfiguration()) + .Then(x => x.ThenTheResultIsNotValid()) + .And(x => x.ThenTheErrorMessageAtPositionIs(0, "ServiceName cannot be empty or null when using service discovery or Ocelot cannot look up your service!")) + .BDDfy(); + } + + [Fact] + public void configuration_is_valid_with_using_service_discovery_and_service_name() + { + this.Given(x => x.GivenAConfiguration(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/api/products/", + UpstreamPathTemplate = "/asdf/", + UpstreamHttpMethod = new List {"Get"}, + UseServiceDiscovery = true, + ServiceName = "Test" + } + } + })) + .When(x => x.WhenIValidateTheConfiguration()) + .Then(x => x.ThenTheResultIsValid()) + .BDDfy(); + } + + + [Theory] + [InlineData(null)] + [InlineData("")] + public void configuration_is_invalid_when_not_using_service_discovery_and_host(string downstreamHost) + { + this.Given(x => x.GivenAConfiguration(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/api/products/", + UpstreamPathTemplate = "/asdf/", + UpstreamHttpMethod = new List {"Get"}, + UseServiceDiscovery = false, + DownstreamHost = downstreamHost + } + } + })) + .When(x => x.WhenIValidateTheConfiguration()) + .Then(x => x.ThenTheResultIsNotValid()) + .And(x => x.ThenTheErrorMessageAtPositionIs(0, "When not using service discover DownstreamHost must be set or Ocelot cannot find your service!")) + .BDDfy(); + } + + [Fact] + public void configuration_is_valid_when_not_using_service_discovery_and_host_is_set() + { + this.Given(x => x.GivenAConfiguration(new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/api/products/", + UpstreamPathTemplate = "/asdf/", + UpstreamHttpMethod = new List {"Get"}, + UseServiceDiscovery = false, + DownstreamHost = "bbc.co.uk" + } + } + })) + .When(x => x.WhenIValidateTheConfiguration()) + .Then(x => x.ThenTheResultIsValid()) .BDDfy(); } @@ -201,6 +418,11 @@ namespace Ocelot.UnitTests.Configuration _result.Data.Errors[0].ShouldBeOfType(); } + private void ThenTheErrorMessageAtPositionIs(int index, string expected) + { + _result.Data.Errors[index].Message.ShouldBe(expected); + } + private void GivenTheAuthSchemeExists(string name) { _provider.Setup(x => x.GetAllSchemesAsync()).ReturnsAsync(new List