fix: Allow empty YAML files to be loaded

- Better contextual logging for YAML files
- When there's a syntax error in file parsing, skip that file.
- When validation fails for instance config, skip just that instance.
- If a file is empty, print a warning and skip it.
- Print instance name (instead of URL) in more places.
pull/151/head
Robert Dailey 1 year ago
parent 51d0219a1a
commit 8a124d12f9

@ -11,6 +11,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- Improved logging: theme changes, better exception handling, more detail written to log files.
- Print instance name instead of URL in more places.
- Configuration parsing is more forgiving about errors:
- If there's a YAML syntax error, skip the file but continue.
- If there's a validation error, skip only that instance (not the whole file).
### Fixed
- Empty configuration files are skipped if they are empty (warning is printed).
## [3.0.0] - 2022-12-03

@ -13,11 +13,11 @@ namespace Recyclarr.Tests.Command.Helpers;
public class CacheStoragePathTest : IntegrationFixture
{
[Test]
public void Use_guid_when_empty_name()
public void Use_guid_when_no_name()
{
var config = Substitute.ForPartsOf<ServiceConfiguration>();
config.BaseUrl = "something";
config.Name = "";
config.Name = null;
using var scope = Container.BeginLifetimeScope(builder =>
{
@ -31,7 +31,7 @@ public class CacheStoragePathTest : IntegrationFixture
}
[Test]
public void Use_name_when_not_empty()
public void Use_name_when_not_null()
{
var config = Substitute.ForPartsOf<ServiceConfiguration>();
config.BaseUrl = "something";

@ -2,7 +2,7 @@ using System.IO.Abstractions;
using System.IO.Abstractions.Extensions;
using System.IO.Abstractions.TestingHelpers;
using System.Text;
using AutoFixture.NUnit3;
using Autofac;
using Common;
using Common.Extensions;
using FluentAssertions;
@ -12,7 +12,10 @@ using NSubstitute;
using NUnit.Framework;
using Recyclarr.Config;
using Recyclarr.TestLibrary;
using Serilog.Sinks.TestCorrelator;
using TestLibrary;
using TestLibrary.AutoFixture;
using TrashLib.Config.Secrets;
using TrashLib.Services.Sonarr.Config;
using TrashLib.TestLibrary;
using YamlDotNet.Core;
@ -29,6 +32,12 @@ public class ConfigurationLoaderTest : IntegrationFixture
return new StringReader(testData.ReadData(file));
}
protected override void RegisterExtraTypes(ContainerBuilder builder)
{
base.RegisterExtraTypes(builder);
builder.RegisterMockFor<IValidator<TestConfig>>();
}
[Test]
public void Load_many_iterations_of_config()
{
@ -44,7 +53,8 @@ public class ConfigurationLoaderTest : IntegrationFixture
var fileData = new (string, string)[]
{
(baseDir.File("config1.yml").FullName, MockYaml(1, 2)),
(baseDir.File("config2.yml").FullName, MockYaml(3))
(baseDir.File("config2.yml").FullName, MockYaml(3)),
(baseDir.File("config3.yml").FullName, "bad yaml")
};
foreach (var (file, data) in fileData)
@ -101,32 +111,43 @@ public class ConfigurationLoaderTest : IntegrationFixture
});
}
[Test, AutoMockData]
public void Throw_when_validation_fails(
[Frozen] IValidator<TestConfig> validator,
ConfigurationLoader<TestConfig> configLoader)
[Test]
public void Skip_when_validation_fails()
// [Frozen] IValidator<TestConfig> validator,
// ConfigurationLoader<TestConfig> configLoader)
{
var validator = Resolve<IValidator<TestConfig>>();
var sut = Resolve<ConfigurationLoader<TestConfig>>();
// force the validator to return a validation error
validator.Validate(Arg.Any<TestConfig>()).Returns(new ValidationResult
{
Errors = {new ValidationFailure("PropertyName", "Test Validation Failure")}
Errors =
{
new ValidationFailure("PropertyName", "Test Validation Failure"),
new ValidationFailure("Another", "This is yet another failure")
}
});
const string testYml = @"
fubar:
- api_key: abc
instance1:
api_key: abc
";
Action act = () => configLoader.LoadFromStream(new StringReader(testYml), "fubar");
var result = sut.LoadFromStream(new StringReader(testYml), "fubar");
act.Should().Throw<ConfigurationException>();
result.Should().BeEmpty();
}
[Test, AutoMockData]
public void Validation_success_does_not_throw(ConfigurationLoader<TestConfig> configLoader)
[Test]
public void Validation_success_does_not_throw()
{
var configLoader = Resolve<ConfigurationLoader<TestConfig>>();
const string testYml = @"
fubar:
- api_key: abc
instanceA:
api_key: abc
";
Action act = () => configLoader.LoadFromStream(new StringReader(testYml), "fubar");
act.Should().NotThrow();
@ -139,9 +160,10 @@ fubar:
const string testYml = @"
sonarr:
- api_key: !secret api_key
base_url: !secret 123GARBAGE_
release_profiles:
instance1:
api_key: !secret api_key
base_url: !secret 123GARBAGE_
release_profiles:
- trash_ids:
- !secret secret_rp
";
@ -157,6 +179,7 @@ secret_rp: 1234567
{
new()
{
Name = "instance1",
ApiKey = "95283e6b156c42f3af8a9b16173f876b",
BaseUrl = "https://radarr:7878",
ReleaseProfiles = new List<ReleaseProfileConfig>
@ -176,48 +199,55 @@ secret_rp: 1234567
[Test]
public void Throw_when_referencing_invalid_secret()
{
using var logContext = TestCorrelator.CreateContext();
var configLoader = Resolve<ConfigurationLoader<SonarrConfiguration>>();
const string testYml = @"
sonarr:
- api_key: !secret api_key
base_url: fake_url
instance2:
api_key: !secret api_key
base_url: fake_url
";
const string secretsYml = @"
no_api_key: 95283e6b156c42f3af8a9b16173f876b
";
const string secretsYml = "no_api_key: 95283e6b156c42f3af8a9b16173f876b";
Fs.AddFile(Paths.SecretsPath.FullName, new MockFileData(secretsYml));
Action act = () => configLoader.LoadFromStream(new StringReader(testYml), "sonarr");
act.Should().Throw<YamlException>().WithMessage("api_key is not defined in secrets.yml.");
var act = () => configLoader.LoadFromStream(new StringReader(testYml), "sonarr");
act.Should().Throw<YamlException>()
.WithInnerException<SecretNotFoundException>()
.WithMessage("*api_key is not defined in secrets.yml");
}
[Test]
public void Throw_when_referencing_secret_without_secrets_file()
{
var configLoader = Resolve<ConfigurationLoader<SonarrConfiguration>>();
var configLoader = Resolve<ConfigurationLoader<TestConfig>>();
const string testYml = @"
sonarr:
- api_key: !secret api_key
base_url: fake_url
instance3:
api_key: !secret api_key
base_url: fake_url
";
Action act = () => configLoader.LoadFromStream(new StringReader(testYml), "sonarr");
act.Should().Throw<YamlException>().WithMessage("api_key is not defined in secrets.yml.");
act.Should().Throw<YamlException>()
.WithInnerException<SecretNotFoundException>()
.WithMessage("*api_key is not defined in secrets.yml");
}
[Test]
public void Throw_when_secret_value_is_not_scalar()
{
var configLoader = Resolve<ConfigurationLoader<SonarrConfiguration>>();
var configLoader = Resolve<ConfigurationLoader<TestConfig>>();
const string testYml = @"
sonarr:
- api_key: !secret { property: value }
base_url: fake_url
instance4:
api_key: !secret { property: value }
base_url: fake_url
";
Action act = () => configLoader.LoadFromStream(new StringReader(testYml), "sonarr");
@ -231,17 +261,26 @@ sonarr:
const string testYml = @"
sonarr:
- api_key: fake_key
base_url: fake_url
release_profiles: !secret bogus_profile
instance5:
api_key: fake_key
base_url: fake_url
release_profiles: !secret bogus_profile
";
const string secretsYml = @"
bogus_profile: 95283e6b156c42f3af8a9b16173f876b
";
const string secretsYml = @"bogus_profile: 95283e6b156c42f3af8a9b16173f876b";
Fs.AddFile(Paths.SecretsPath.FullName, new MockFileData(secretsYml));
Action act = () => configLoader.LoadFromStream(new StringReader(testYml), "sonarr");
act.Should().Throw<YamlException>().WithMessage("Exception during deserialization");
}
[Test, AutoMockData]
public void Yaml_file_with_only_comment_should_be_skipped(ConfigurationLoader<TestConfig> sut)
{
const string testYml = "# YAML with nothing but this comment";
var result = sut.LoadFromStream(new StringReader(testYml), "fubar");
result.Should().BeEmpty();
}
}

@ -34,7 +34,7 @@ public class CacheStoragePath : ICacheStoragePath
{
return _paths.CacheDirectory
.SubDirectory(_serviceCommand.Name.ToLower())
.SubDirectory(_config.Name.Any() ? _config.Name : BuildServiceGuid())
.SubDirectory(_config.Name ?? BuildServiceGuid())
.File(cacheObjectName + ".json");
}
}

@ -60,7 +60,8 @@ internal class RadarrCommand : ServiceCommand
builder.RegisterInstance(config).As<IServiceConfiguration>();
});
log.Information("Processing server {Url}", FlurlLogging.SanitizeUrl(config.BaseUrl));
log.Information("Processing {Server} server {Name}",
Name, config.Name ?? FlurlLogging.SanitizeUrl(config.BaseUrl));
// There's no actual compatibility checks to perform yet. We directly access the RadarrCompatibility class,
// as opposed to a IRadarrVersionEnforcement object (like Sonarr does), simply to force the API invocation

@ -94,7 +94,8 @@ public class SonarrCommand : ServiceCommand
builder.RegisterInstance(config).As<IServiceConfiguration>();
});
log.Information("Processing server {Url}", FlurlLogging.SanitizeUrl(config.BaseUrl));
log.Information("Processing {Server} server {Name}",
Name, config.Name ?? FlurlLogging.SanitizeUrl(config.BaseUrl));
var versionEnforcement = scope.Resolve<ISonarrVersionEnforcement>();
await versionEnforcement.DoVersionEnforcement(config);

@ -57,7 +57,6 @@ public class ConfigurationFinder : IConfigurationFinder
throw new CommandException("No configuration YAML files found");
}
_log.Debug("Using config files: {ConfigFiles}", configs);
return configs;
}
}

@ -1,8 +1,11 @@
using System.IO.Abstractions;
using FluentValidation;
using Recyclarr.Logging;
using Serilog;
using Serilog.Context;
using TrashLib.Config;
using TrashLib.Config.Services;
using TrashLib.Http;
using YamlDotNet.Core;
using YamlDotNet.Core.Events;
using YamlDotNet.Serialization;
@ -14,95 +17,159 @@ public class ConfigurationLoader<T> : IConfigurationLoader<T>
{
private readonly ILogger _log;
private readonly IDeserializer _deserializer;
private readonly IFileSystem _fileSystem;
private readonly IFileSystem _fs;
private readonly IValidator<T> _validator;
public ConfigurationLoader(
ILogger log,
IFileSystem fileSystem,
IFileSystem fs,
IYamlSerializerFactory yamlFactory,
IValidator<T> validator)
{
_log = log;
_fileSystem = fileSystem;
_fs = fs;
_validator = validator;
_deserializer = yamlFactory.CreateDeserializer();
}
public IEnumerable<T> Load(string file, string configSection)
public ICollection<T> LoadMany(IEnumerable<string> configFiles, string configSection)
{
using var stream = _fileSystem.File.OpenText(file);
return LoadFromStream(stream, configSection);
return configFiles.SelectMany(file => Load(file, configSection)).ToList();
}
public IEnumerable<T> LoadFromStream(TextReader stream, string configSection)
public ICollection<T> Load(string file, string configSection)
{
var parser = new Parser(stream);
parser.Consume<StreamStart>();
parser.Consume<DocumentStart>();
parser.Consume<MappingStart>();
_log.Debug("Loading config file: {File}", file);
using var logScope = LogContext.PushProperty(LogProperty.Scope, _fs.Path.GetFileName(file));
var validConfigs = new List<T>();
while (parser.TryConsume<Scalar>(out var key))
try
{
if (key.Value != configSection)
using var stream = _fs.File.OpenText(file);
var configs = LoadFromStream(stream, configSection);
if (!configs.Any())
{
parser.SkipThisAndNestedEvents();
continue;
_log.Warning("Configuration file yielded no usable configuration (is it empty?)");
}
List<T>? configs;
switch (parser.Current)
return configs;
}
catch (YamlException e)
{
var line = e.Start.Line;
switch (e.InnerException)
{
case MappingStart:
configs = _deserializer.Deserialize<Dictionary<string, T>>(parser)
.Select(kvp =>
{
kvp.Value.Name = kvp.Key;
return kvp.Value;
})
.ToList();
break;
case SequenceStart:
_log.Warning(
"Found array-style list of instances instead of named-style. Array-style lists of Sonarr/Radarr " +
"instances are deprecated");
configs = _deserializer.Deserialize<List<T>>(parser);
case InvalidCastException:
_log.Error("Incompatible value assigned/used at line {Line}", line);
break;
default:
configs = null;
_log.Error("Exception at line {Line}: {Msg}", line, e.InnerException?.Message ?? e.Message);
break;
}
}
if (configs is not null)
{
ValidateConfigs(configSection, configs, validConfigs);
}
_log.Error("Due to previous exception, this file will be skipped: {File}", file);
return Array.Empty<T>();
}
public ICollection<T> LoadFromStream(TextReader stream, string requestedSection)
{
_log.Debug("Loading config section: {Section}", requestedSection);
var parser = new Parser(stream);
parser.SkipThisAndNestedEvents();
parser.Consume<StreamStart>();
if (parser.Current is StreamEnd)
{
_log.Debug("Skipping this config due to StreamEnd");
return Array.Empty<T>();
}
return validConfigs;
parser.Consume<DocumentStart>();
if (parser.Current is DocumentEnd)
{
_log.Debug("Skipping this config due to DocumentEnd");
return Array.Empty<T>();
}
return ParseAllSections(parser, requestedSection);
}
private void ValidateConfigs(string configSection, IEnumerable<T> configs, ICollection<T> validConfigs)
private ICollection<T> ParseAllSections(Parser parser, string requestedSection)
{
foreach (var config in configs)
var configs = new List<T>();
parser.Consume<MappingStart>();
while (parser.TryConsume<Scalar>(out var section))
{
var result = _validator.Validate(config);
if (result is {IsValid: false})
if (section.Value == requestedSection)
{
throw new ConfigurationException(configSection, typeof(T), result.Errors);
configs.AddRange(ParseSingleSection(parser));
}
else
{
_log.Debug("Skipping non-matching config section {Section} at line {Line}",
section.Value, section.Start.Line);
parser.SkipThisAndNestedEvents();
}
}
// If any config names are null, that means user specified array-style (deprecated) instances.
if (configs.Any(x => x.Name is null))
{
_log.Warning(
"Found array-style list of instances instead of named-style. " +
"Array-style lists of Sonarr/Radarr instances are deprecated");
}
return configs;
}
private ICollection<T> ParseSingleSection(Parser parser)
{
var configs = new List<T>();
switch (parser.Current)
{
case MappingStart:
ParseAndAdd<MappingStart, MappingEnd>(parser, configs);
break;
validConfigs.Add(config);
case SequenceStart:
ParseAndAdd<SequenceStart, SequenceEnd>(parser, configs);
break;
}
return configs;
}
public IEnumerable<T> LoadMany(IEnumerable<string> configFiles, string configSection)
private void ParseAndAdd<TStart, TEnd>(Parser parser, ICollection<T> configs)
where TStart : ParsingEvent
where TEnd : ParsingEvent
{
return configFiles.SelectMany(file => Load(file, configSection));
parser.Consume<TStart>();
while (!parser.TryConsume<TEnd>(out _))
{
var lineNumber = parser.Current?.Start.Line;
string? instanceName = null;
if (parser.TryConsume<Scalar>(out var key))
{
instanceName = key.Value;
}
var newConfig = _deserializer.Deserialize<T>(parser);
newConfig.Name = instanceName;
var result = _validator.Validate(newConfig);
if (result is {IsValid: false})
{
var printableName = instanceName ?? FlurlLogging.SanitizeUrl(newConfig.BaseUrl);
_log.Error("Validation failed for instance config {Instance} at line {Line} with errors {Errors}",
printableName, lineNumber, result.Errors);
continue;
}
configs.Add(newConfig);
}
}
}

@ -2,8 +2,10 @@ using TrashLib.Config.Services;
namespace Recyclarr.Config;
public interface IConfigurationLoader<out T>
public interface IConfigurationLoader<T>
where T : IServiceConfiguration
{
IEnumerable<T> LoadMany(IEnumerable<string> configFiles, string configSection);
ICollection<T> LoadMany(IEnumerable<string> configFiles, string configSection);
ICollection<T> Load(string file, string configSection);
ICollection<T> LoadFromStream(TextReader stream, string requestedSection);
}

@ -0,0 +1,6 @@
namespace Recyclarr.Logging;
public static class LogProperty
{
public static string Scope => nameof(Scope);
}

@ -11,31 +11,31 @@ public class LoggerFactory
{
private readonly IAppPaths _paths;
private const string BaseTemplate =
"{@m}" +
"{@x}" +
"\n";
public LoggerFactory(IAppPaths paths)
{
_paths = paths;
}
private static ExpressionTemplate GetConsoleTemplate()
private static string GetBaseTemplateString()
{
const string template =
"[{@l:u3}] " +
BaseTemplate;
var scope = LogProperty.Scope;
return
$"{{#if {scope} is not null}}{{{scope}}}: {{#end}}" +
"{@m}" +
"{@x}" +
"\n";
}
private static ExpressionTemplate GetConsoleTemplate()
{
var template = "[{@l:u3}] " + GetBaseTemplateString();
return new ExpressionTemplate(template, theme: TemplateTheme.Code);
}
private static ExpressionTemplate GetFileTemplate()
{
const string template =
"[{@t:HH:mm:ss} {@l:u3}] " +
BaseTemplate;
var template = "[{@t:HH:mm:ss} {@l:u3}] " + GetBaseTemplateString();
return new ExpressionTemplate(template);
}
@ -47,6 +47,7 @@ public class LoggerFactory
.MinimumLevel.Is(LogEventLevel.Debug)
.WriteTo.Console(GetConsoleTemplate(), level)
.WriteTo.File(GetFileTemplate(), logPath.FullName)
.Enrich.FromLogContext()
.CreateLogger();
}
}

@ -0,0 +1,18 @@
using FluentAssertions;
using NUnit.Framework;
using TrashLib.Config.Secrets;
namespace TrashLib.Tests.Config.Secrets;
[TestFixture]
[Parallelizable(ParallelScope.All)]
public class SecretNotFoundExceptionTest
{
[Test]
public void Properties_get_initialized()
{
var sut = new SecretNotFoundException(15, "key");
sut.Line.Should().Be(15);
sut.SecretKey.Should().Be("key");
}
}

@ -4,6 +4,6 @@ namespace TrashLib.Config;
public interface IYamlSerializerFactory
{
IDeserializer CreateDeserializer();
IDeserializer CreateDeserializer(Action<DeserializerBuilder>? extraBuilder = null);
ISerializer CreateSerializer();
}

@ -0,0 +1,14 @@
namespace TrashLib.Config.Secrets;
public class SecretNotFoundException : Exception
{
public int Line { get; }
public string SecretKey { get; }
public SecretNotFoundException(int line, string secretKey)
: base($"Secret used on line {line} with key {secretKey} is not defined in secrets.yml")
{
Line = line;
SecretKey = secretKey;
}
}

@ -25,10 +25,10 @@ public class SecretsDeserializer : INodeDeserializer
return false;
}
var scalar = reader.Consume<Scalar>();
if (!_secrets.Secrets.TryGetValue(scalar.Value, out var secretsValue))
var secretKey = reader.Consume<Scalar>();
if (!_secrets.Secrets.TryGetValue(secretKey.Value, out var secretsValue))
{
throw new YamlException(scalar.Start, scalar.End, $"{scalar.Value} is not defined in secrets.yml.");
throw new SecretNotFoundException(secretKey.Start.Line, secretKey.Value);
}
value = secretsValue;

@ -2,7 +2,7 @@ namespace TrashLib.Config.Services;
public interface IServiceConfiguration
{
string Name { get; }
string? Name { get; }
string BaseUrl { get; }
string ApiKey { get; }
ICollection<CustomFormatConfig> CustomFormats { get; }

@ -1,11 +1,14 @@
using JetBrains.Annotations;
using YamlDotNet.Serialization;
namespace TrashLib.Config.Services;
public abstract class ServiceConfiguration : IServiceConfiguration
{
// Name is set dynamically by
public string Name { get; set; } = "";
// Name is set dynamically by ConfigurationLoader
[YamlIgnore]
public string? Name { get; set; }
public string BaseUrl { get; set; } = "";
public string ApiKey { get; set; } = "";

@ -16,7 +16,7 @@ public class YamlSerializerFactory : IYamlSerializerFactory
_secretsProvider = secretsProvider;
}
public IDeserializer CreateDeserializer()
public IDeserializer CreateDeserializer(Action<DeserializerBuilder>? extraBuilder = null)
{
var builder = new DeserializerBuilder()
.WithNamingConvention(UnderscoredNamingConvention.Instance)
@ -27,6 +27,8 @@ public class YamlSerializerFactory : IYamlSerializerFactory
.WithNodeDeserializer(new ForceEmptySequences(_objectFactory))
.WithObjectFactory(_objectFactory);
extraBuilder?.Invoke(builder);
return builder.Build();
}

@ -40,7 +40,6 @@ public class CustomFormatLoader : ICustomFormatLoader
IReadOnlyCollection<CustomFormatCategoryItem> categories)
{
return Observable.Using(file.OpenText, x => x.ReadToEndAsync().ToObservable())
.Do(_ => _log.Debug("Parsing CF Json: {Name}", file.Name))
.Select(x =>
{
var cf = _parser.ParseCustomFormatData(x, file.Name);

Loading…
Cancel
Save