Ensure that schedule is a valid Cron expression

changelog/content/experimental/unreleased.md

@@ -8,4 +8,5 @@ version:
 - {{% tag added %}} New validation rule to ensure at least one resource or resource collection is configured to scrape
 - {{% tag removed %}} Support for Swagger 2.0 ([deprecation notice](https://changelog.promitor.io/#swagger-2-0))
 - {{% tag removed %}} Support for Swagger UI 2.0 ([deprecation notice](https://changelog.promitor.io/#swagger-ui-2-0))
-- {{% tag added %}} Provide suggestions when unknown fields are found in the metrics config. [#1105](https://github.com/tomkerkhove/promitor/issues/1105).
+- {{% tag added %}} Provide suggestions when unknown fields are found in the metrics config. [#1105](https://github.com/tomkerkhove/promitor/issues/1105).
+- {{% tag added %}} Add validation to ensure the scraping schedule is a valid Cron expression. [#1103](https://github.com/tomkerkhove/promitor/issues/1103).

src/Promitor.Core.Scraping/Configuration/Serialization/Deserializer.cs

@@ -42,6 +42,7 @@ public virtual TObject Deserialize(YamlMappingNode node, IErrorReporter errorRep
                 {
                     fieldContext.SetValue(
                         result, GetFieldValue(child, fieldContext.DeserializationInfo, errorReporter));
+                    fieldContext.Validate(child, errorReporter);
                 }
                 else
                 {

src/Promitor.Core.Scraping/Configuration/Serialization/FieldDeserializationContext.cs

@@ -1,4 +1,6 @@
-using GuardNet;
+using System.Collections.Generic;
+using GuardNet;
+using YamlDotNet.RepresentationModel;
 
 namespace Promitor.Core.Scraping.Configuration.Serialization
 {
@@ -41,6 +43,19 @@ public void SetValue(TObject target, object value)
             HasBeenSet = true;
         }
 
+        /// <summary>
+        /// Runs any custom validators that have been defined for the field.
+        /// </summary>
+        /// <param name="fieldNodes">The pair of nodes defining the name of the field, and its value.</param>
+        /// <param name="errorReporter">Used to report any validation errors.</param>
+        public void Validate(KeyValuePair<YamlNode, YamlNode> fieldNodes, IErrorReporter errorReporter)
+        {
+            foreach (var validator in DeserializationInfo.Validators)
+            {
+                validator.Validate(fieldNodes.Value.ToString(), fieldNodes, errorReporter);
+            }
+        }
+
         /// <summary>
         /// Sets the field to its default value.
         /// </summary>

src/Promitor.Core.Scraping/Configuration/Serialization/FieldDeserializationInfo.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Reflection;
+using Promitor.Core.Scraping.Configuration.Serialization.FieldValidators;
 using YamlDotNet.RepresentationModel;
 
 namespace Promitor.Core.Scraping.Configuration.Serialization
@@ -20,14 +21,22 @@ public class FieldDeserializationInfo
         /// <param name="defaultValue">The default value to use for the field if not specified.</param>
         /// <param name="customMapperFunc">A custom function to use for getting the value of the field.</param>
         /// <param name="deserializer">A deserializer to use to deserialize the field.</param>
-        public FieldDeserializationInfo(PropertyInfo propertyInfo, bool isRequired, object defaultValue, Func<string, KeyValuePair<YamlNode, YamlNode>, IErrorReporter, object> customMapperFunc, IDeserializer deserializer = null)
+        /// <param name="validators">Any custom validators to validate the field using.</param>
+        public FieldDeserializationInfo(
+            PropertyInfo propertyInfo,
+            bool isRequired,
+            object defaultValue,
+            Func<string, KeyValuePair<YamlNode, YamlNode>, IErrorReporter, object> customMapperFunc,
+            IDeserializer deserializer,
+            IReadOnlyCollection<IFieldValidator> validators)
         {
             YamlFieldName = GetName(propertyInfo);
             CustomMapperFunc = customMapperFunc;
             PropertyInfo = propertyInfo;
             IsRequired = isRequired;
             DefaultValue = defaultValue;
             Deserializer = deserializer;
+            Validators = validators;
         }
 
         /// <summary>
@@ -60,6 +69,11 @@ public FieldDeserializationInfo(PropertyInfo propertyInfo, bool isRequired, obje
         /// </summary>
         public IDeserializer Deserializer { get; }
 
+        /// <summary>
+        /// Gets the custom validators for the field.
+        /// </summary>
+        public IReadOnlyCollection<IFieldValidator> Validators { get; }
+
         private static string GetName(MemberInfo propertyInfo)
         {
             return char.ToLowerInvariant(propertyInfo.Name[0]) + propertyInfo.Name.Substring(1);

src/Promitor.Core.Scraping/Configuration/Serialization/FieldDeserializationInfoBuilder.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Linq.Expressions;
 using System.Reflection;
+using Promitor.Core.Scraping.Configuration.Serialization.FieldValidators;
 using YamlDotNet.RepresentationModel;
 
 namespace Promitor.Core.Scraping.Configuration.Serialization
@@ -11,6 +12,7 @@ namespace Promitor.Core.Scraping.Configuration.Serialization
     /// </summary>
     public class FieldDeserializationInfoBuilder<TObject, TReturn> : IFieldDeserializationInfoBuilder
     {
+        private readonly List<IFieldValidator> _validators = new List<IFieldValidator>();
         private PropertyInfo _propertyInfo;
         private bool _isRequired;
         private object _defaultValue;
@@ -88,7 +90,15 @@ public FieldDeserializationInfoBuilder<TObject, TReturn> MapUsingDeserializer(ID
         public FieldDeserializationInfo Build()
         {
             return new FieldDeserializationInfo(
-                _propertyInfo, _isRequired, _defaultValue, _customMapperFunc, _deserializer);
+                _propertyInfo, _isRequired, _defaultValue, _customMapperFunc, _deserializer, _validators);
+        }
+
+        /// <summary>
+        /// Indicates that this field should be validated as a Cron expression.
+        /// </summary>
+        public void ValidateCronExpression()
+        {
+            _validators.Add(new CronExpressionValidator());
         }
     }
 }

src/Promitor.Core.Scraping/Configuration/Serialization/FieldValidators/CronExpressionValidator.cs

@@ -0,0 +1,50 @@
+using System.Collections.Generic;
+using Cronos;
+using YamlDotNet.RepresentationModel;
+
+namespace Promitor.Core.Scraping.Configuration.Serialization.FieldValidators
+{
+    /// <summary>
+    /// Ensures that a field is a valid Cron expression.
+    /// </summary>
+    public class CronExpressionValidator : IFieldValidator
+    {
+        /// <inheritdoc />
+        public void Validate(string value, KeyValuePair<YamlNode, YamlNode> fieldNodes, IErrorReporter errorReporter)
+        {
+            if (!string.IsNullOrEmpty(value))
+            {
+                if (!IsValidStandardExpression(value) && !IsValidExpressionWithSeconds(value))
+                {
+                    errorReporter.ReportError(fieldNodes.Value, $"'{value}' is not a valid value for '{fieldNodes.Key}'. The value must be a valid Cron expression.");
+                }
+            }
+        }
+
+        private static bool IsValidStandardExpression(string value)
+        {
+            try
+            {
+                CronExpression.Parse(value);
+                return true;
+            }
+            catch (CronFormatException)
+            {
+                return false;
+            }
+        }
+
+        private static bool IsValidExpressionWithSeconds(string value)
+        {
+            try
+            {
+                CronExpression.Parse(value, CronFormat.IncludeSeconds);
+                return true;
+            }
+            catch (CronFormatException)
+            {
+                return false;
+            }
+        }
+    }
+}

src/Promitor.Core.Scraping/Configuration/Serialization/FieldValidators/IFieldValidator.cs

@@ -0,0 +1,19 @@
+using System.Collections.Generic;
+using YamlDotNet.RepresentationModel;
+
+namespace Promitor.Core.Scraping.Configuration.Serialization.FieldValidators
+{
+    /// <summary>
+    /// Provides validation for a yaml field.
+    /// </summary>
+    public interface IFieldValidator
+    {
+        /// <summary>
+        /// Validates a field, reporting any errors via the error reporter.
+        /// </summary>
+        /// <param name="value">The value of the field.</param>
+        /// <param name="fieldNodes">A KeyValuePair where the Key is the field name, and the Value is its value.</param>
+        /// <param name="errorReporter">The error reporter.</param>
+        void Validate(string value, KeyValuePair<YamlNode, YamlNode> fieldNodes, IErrorReporter errorReporter);
+    }
+}

src/Promitor.Core.Scraping/Configuration/Serialization/v1/Core/ScrapingDeserializer.cs

@@ -8,7 +8,8 @@ public class ScrapingDeserializer : Deserializer<ScrapingV1>
         public ScrapingDeserializer(ILogger<ScrapingDeserializer> logger) : base(logger)
         {
             Map(scraping => scraping.Schedule)
-                .IsRequired();
+                .IsRequired()
+                .ValidateCronExpression();
         }
     }
 }

src/Promitor.Core.Scraping/Promitor.Core.Scraping.csproj

@@ -17,6 +17,7 @@
   <ItemGroup>
     <PackageReference Include="AutoMapper" Version="9.0.0" />
     <PackageReference Include="Fastenshtein" Version="1.0.0.5" />
+    <PackageReference Include="Cronos" Version="0.7.0" />
     <PackageReference Include="Guard.Net" Version="1.2.0" />
     <PackageReference Include="Microsoft.ApplicationInsights" Version="2.14.0" />
     <PackageReference Include="Microsoft.Azure.Management.Fluent" Version="1.34.0" />

src/Promitor.Tests.Unit/Serialization/DeserializationContextTests.cs

@@ -1,6 +1,7 @@
 using System.Collections.Generic;
 using System.Reflection;
 using Promitor.Core.Scraping.Configuration.Serialization;
+using Promitor.Core.Scraping.Configuration.Serialization.FieldValidators;
 using Xunit;
 
 namespace Promitor.Tests.Unit.Serialization
@@ -88,7 +89,7 @@ public void GetSuggestions_IgnoredFields_IncludesIgnoredFieldsInSuggestions()
 
         private FieldDeserializationInfo CreateField(PropertyInfo propertyInfo)
         {
-            return new FieldDeserializationInfo(propertyInfo, false, null, null, null);
+            return new FieldDeserializationInfo(propertyInfo, false, null, null, null, new List<IFieldValidator>());
         }
 
         public class Person

src/Promitor.Tests.Unit/Serialization/DeserializerTests/ValidationTests.cs

@@ -168,6 +168,21 @@ public void Deserialize_UnknownField_ReturnsSuggestion()
             _errorReporter.Verify(r => r.ReportWarning(nameTagNode, "Unknown field 'nmae'. Did you mean 'name'?"));
         }
 
+        [Fact]
+        public void Deserialize_CronSyntaxInvalid_ReportsError()
+        {
+            // Arrange
+            var node = YamlUtils.CreateYamlNode("schedule: 12345");
+            var dayValueNode = node.Children.Single(c => c.Key.ToString() == "schedule").Value;
+
+            // Act / Assert
+            YamlAssert.ReportsError(
+                _deserializer,
+                node,
+                dayValueNode,
+                "'12345' is not a valid value for 'schedule'. The value must be a valid Cron expression.");
+        }
+
         [Fact]
         public void Deserialize_UnknownField_ReturnsMultipleSuggestions()
         {
@@ -190,6 +205,7 @@ private class TestConfigObject
             public DayOfWeek Day { get; set; }
             public string Date { get; set; }
             public TimeSpan Interval { get; set; }
+            public string Schedule { get; set; }
         }
 
         private class TestDeserializer: Deserializer<TestConfigObject>
@@ -201,6 +217,8 @@ public TestDeserializer() : base(NullLogger.Instance)
                 Map(t => t.Day);
                 Map(t => t.Date);
                 Map(t => t.Interval);
+                Map(t => t.Schedule)
+                    .ValidateCronExpression();
                 IgnoreField("customField");
             }
         }

src/Promitor.Tests.Unit/Serialization/FieldValidators/CronExpressionValidatorTests.cs

@@ -0,0 +1,72 @@
+using System.Linq;
+using Moq;
+using Promitor.Core.Scraping.Configuration.Serialization;
+using Promitor.Core.Scraping.Configuration.Serialization.FieldValidators;
+using Xunit;
+using YamlDotNet.RepresentationModel;
+
+namespace Promitor.Tests.Unit.Serialization.FieldValidators
+{
+    public class CronExpressionValidatorTests
+    {
+        private readonly CronExpressionValidator _validator = new CronExpressionValidator();
+
+        [Fact]
+        public void Validate_InvalidExpression_ReportsError()
+        {
+            AssertExpressionIsNotValid("invalid-expression");
+        }
+
+        [Fact]
+        public void Validate_ValidExpression_DoesNotReportError()
+        {
+            AssertExpressionIsValid("*/5 * * * *");
+        }
+
+        [Fact]
+        public void Validate_ValidExpressionWithSeconds_DoesNotReportError()
+        {
+            AssertExpressionIsValid("* */5 * * * *");
+        }
+
+        [Fact]
+        public void Validate_EmptyString_DoesNotReportError()
+        {
+            AssertExpressionIsValid(string.Empty);
+        }
+
+        private void AssertExpressionIsNotValid(string cronExpression)
+        {
+            // Arrange
+            var fieldNodes = YamlUtils
+                .CreateYamlNode($"schedule: '{cronExpression}'")
+                .Children
+                .First();
+            var errorReporter = new Mock<IErrorReporter>();
+
+            // Act
+            _validator.Validate(fieldNodes.Value.ToString(), fieldNodes, errorReporter.Object);
+
+            // Assert
+            errorReporter.Verify(
+                e => e.ReportError(fieldNodes.Value, $"'{cronExpression}' is not a valid value for 'schedule'. The value must be a valid Cron expression."));
+        }
+
+        private void AssertExpressionIsValid(string cronExpression)
+        {
+            // Arrange
+            var fieldNodes = YamlUtils
+                .CreateYamlNode($"schedule: '{cronExpression}'")
+                .Children
+                .First();
+            var errorReporter = new Mock<IErrorReporter>();
+
+            // Act
+            _validator.Validate(fieldNodes.Value.ToString(), fieldNodes, errorReporter.Object);
+
+            // Assert
+            errorReporter.Verify(
+                e => e.ReportError(It.IsAny<YamlNode>(), It.IsAny<string>()), Times.Never);
+        }
+    }
+}