Skip to content

Commit 7039ecf

Browse files
committed
(MODULES-10893) Fix Last Day Of Month Trigger
Prior to this change, attempting to create a trigger that will run on the last day of each selected month will fail. The error claims that a a value is out of range ``` OLE error code:0 in <Unknown> <No Description> HRESULT error code:0x8002000a Out of present range. ``` Microsoft [documentation] claims in one place that the correct way to specify the last day of a month is to effectively translate a bitmask value of day 32 and pass that into the `{get:set}_DaysOfMonth` method on the trigger object. The module was previously coded to do exactly this. The provider and it's helpers would catch the string value 'last' in the `on` property of a `monthly` trigger and translate it into the bit mask. This is what resulted in the error, because it turns out that the position for day 32 in the bit mast is outside the acceptable range of values for this property. This PR changes strategy and instead relies on a different section of the documentation in which an entirely different property on the trigger object called [RunOnLastDayOfMonth] is set. The code now catches the string value 'last' in the `on` property of the trigger, and sets this boolean on the trigger object. Testing has shown this to be an effective method of managing this property of a trigger. [documentation]: https://docs.microsoft.com/en-gb/windows/win32/api/taskschd/nf-taskschd-imonthlytrigger-get_daysofmonth [RunOnLastDayOfMonth]: https://docs.microsoft.com/en-gb/windows/win32/api/taskschd/nf-taskschd-imonthlytrigger-put_runonlastdayofmonth
1 parent 106d092 commit 7039ecf

File tree

6 files changed

+400
-344
lines changed

6 files changed

+400
-344
lines changed

.travis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ before_install:
1313
- gem --version
1414
- bundle -v
1515
script:
16-
- 'SIMPLECOV=yes bundle exec rake $CHECK'
16+
- 'bundle exec rake $CHECK'
1717
bundler_args: --without system_tests
1818
rvm:
1919
- 2.5.7

README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,9 @@ For all triggers:
301301
Each month must be an integer between 1 and 12.
302302
* `on` (Required) — Which days of the month the task should run, as an array.
303303
Each day must be an integer between 1 and 31.
304+
* The string `last` may be used in the array for this property to trigger a
305+
task to run on the last day of each selected month. This feature is only
306+
available for tasks with compatibility level `2` or higher.
304307
* For monthly (by weekday) triggers:
305308
* `months` — Which months the task should run, as an array. Defaults to all months. Each month must be an integer between 1 and 12.
306309
* `day_of_week` (Required) — Which day of the week the task should run, as an array with only one element.

lib/puppet/type/scheduled_task.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ def insync?(current)
163163
all months. Each month must be an integer between 1 and 12.
164164
* `on` **(Required)** --- Which days of the month the task should run,
165165
as an array. Each day must be an integer between 1 and 31 or the string `last`.
166+
The string `last` is only supported for tasks with level 2 compatibility or higher.
166167
* For `monthly` (by weekday) triggers:
167168
* `months` --- Which months the task should run, as an array. Defaults to
168169
all months. Each month must be an integer between 1 and 12.

lib/puppet_x/puppetlabs/scheduled_task/trigger.rb

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
# @api private
66
# PuppetX::PuppetLabs::ScheduledTask module
77
module PuppetX::PuppetLabs::ScheduledTask
8-
# This module is used to manage the triggers for the Task Scheduler V2 API
8+
# @api private
9+
# PuppetX::PuppetLabs::ScheduledTask::Trigger module
910
module Trigger
1011
# Gets or sets the amount of time that is allowed to complete the task.
1112
class Duration
@@ -389,42 +390,46 @@ def self.bitmask_to_names(bitmask)
389390

390391
# Defines the day of the month the task will run.
391392
class Days
392-
# 32 bits for 31 possible days to set + value 'last'
393-
MAX_VALUE = 0b11111111111111111111111111111111
393+
# 32 bit mask, but only 31 days can be set.
394+
# This is contrary to the V2 IMonthlyTrigger::DaysOfMonth documentation
395+
# referenced below, but in testing, setting the last bit of the bit
396+
# mask does not set 'last' day of month as that documentation suggests.
397+
# Instead it results in an error. That feature will be handled instead
398+
# by the RunOnLastDayOfMonth property of the trigger object.
399+
# V2 IMonthlyTrigger::RunOnLastDayOfMonth
400+
# https://docs.microsoft.com/en-us/windows/win32/api/taskschd/nf-taskschd-imonthlytrigger-put_runonlastdayofmonth
401+
MAX_VALUE = 0b01111111111111111111111111111111
394402

395403
# V1 MONTHLYDATE structure
396404
# https://msdn.microsoft.com/en-us/library/windows/desktop/aa381918(v=vs.85).aspx
397405
# V2 IMonthlyTrigger::DaysOfMonth
398406
# https://msdn.microsoft.com/en-us/library/windows/desktop/aa380735(v=vs.85).aspx
399407
def self.indexes_to_bitmask(day_indexes)
400-
day_indexes = [day_indexes].flatten.map do |m|
401-
# The special "day" of 'last' is represented by day "number"
402-
# 32. 'last' has the special meaning of "the last day of the
403-
# month", no matter how many days there are in the month.
404-
# raises if unable to convert
405-
(m.is_a?(String) && m.casecmp('last').zero?) ? 32 : Integer(m)
408+
if day_indexes.nil? || (day_indexes.is_a?(Hash) && day_indexes.empty?)
409+
raise TypeError, 'Day indexes value must not be nil or an empty hash.'
406410
end
407411

408-
invalid_days = day_indexes.reject { |i| i.between?(1, 32) }
412+
integer_days = [day_indexes].flatten.select { |i| i.is_a?(Integer) }
413+
invalid_days = integer_days.reject { |i| i.between?(1, 31) }
414+
409415
unless invalid_days.empty?
410-
raise ArgumentError, "Day indexes value #{invalid_days.join(', ')} is invalid. Integers must be in the range 1-31, or 'last'"
416+
raise ArgumentError, "Day indexes value #{invalid_days.join(', ')} is invalid. Integers must be in the range 1-31"
411417
end
412-
413-
day_indexes.reduce(0) { |bitmask, day_index| bitmask | 1 << day_index - 1 }
418+
integer_days.reduce(0) { |bitmask, day_index| bitmask | 1 << day_index - 1 }
414419
end
415420

416421
# Converts bitmask to index
417-
def self.bitmask_to_indexes(bitmask)
422+
def self.bitmask_to_indexes(bitmask, run_on_last_day_of_month = nil)
418423
bitmask = Integer(bitmask)
419424
if bitmask.negative? || bitmask > MAX_VALUE
420425
raise ArgumentError, "bitmask must be specified as an integer from 0 to #{MAX_VALUE.to_s(10)}"
421426
end
422427

423-
bit_index(bitmask).map do |bit_index|
424-
# Day 32 has the special meaning of "the last day of the
425-
# month", no matter how many days there are in the month.
426-
(bit_index == 31) ? 'last' : bit_index + 1
427-
end
428+
indexes = bit_index(bitmask).map { |bit_index| bit_index + 1 }
429+
430+
indexes << 'last' if run_on_last_day_of_month
431+
432+
indexes
428433
end
429434

430435
# Returns bit index
@@ -435,6 +440,14 @@ def self.bit_index(bitmask)
435440
(bitmask & bit_to_check) == bit_to_check
436441
end
437442
end
443+
444+
def self.last_day_of_month?(day_indexes)
445+
invalid_day_names = [day_indexes].flatten.select { |i| i.is_a?(String) && (i != 'last') }
446+
unless invalid_day_names.empty?
447+
raise ArgumentError, "Only 'last' is allowed as a day name. All other values must be integers between 1 and 31."
448+
end
449+
[day_indexes].flatten.include? 'last'
450+
end
438451
end
439452

440453
# Gets or sets the months of the year during which the task runs.
@@ -705,16 +718,16 @@ def self.to_manifest_hash(i_trigger)
705718
manifest_hash['schedule'] = 'daily'
706719
manifest_hash['every'] = i_trigger.DaysInterval
707720
when Type::TASK_TRIGGER_WEEKLY
708-
manifest_hash.merge!('schedule' => 'weekly',
721+
manifest_hash.merge!('schedule' => 'weekly',
709722
'every' => i_trigger.WeeksInterval,
710723
'day_of_week' => Day.bitmask_to_names(i_trigger.DaysOfWeek))
711724
when Type::TASK_TRIGGER_MONTHLY
712725
manifest_hash.merge!('schedule' => 'monthly',
713726
'months' => Month.bitmask_to_indexes(i_trigger.MonthsOfYear),
714-
'on' => Days.bitmask_to_indexes(i_trigger.DaysOfMonth))
727+
'on' => Days.bitmask_to_indexes(i_trigger.DaysOfMonth, i_trigger.RunOnLastDayOfMonth))
715728
when Type::TASK_TRIGGER_MONTHLYDOW
716729
occurrences = V2::WeeksOfMonth.bitmask_to_names(i_trigger.WeeksOfMonth)
717-
manifest_hash.merge!('schedule' => 'monthly',
730+
manifest_hash.merge!('schedule' => 'monthly',
718731
'months' => Month.bitmask_to_indexes(i_trigger.MonthsOfYear),
719732
# HACK: choose only the first week selected when converting - this LOSES information
720733
'which_occurrence' => occurrences.first || '',
@@ -783,6 +796,7 @@ def self.append_trigger(definition, manifest_hash)
783796

784797
when Type::TASK_TRIGGER_MONTHLY
785798
# https://msdn.microsoft.com/en-us/library/windows/desktop/aa382062(v=vs.85).aspx
799+
i_trigger.RunOnLastDayOfMonth = Days.last_day_of_month?(manifest_hash['on'])
786800
i_trigger.DaysOfMonth = Days.indexes_to_bitmask(manifest_hash['on'])
787801
i_trigger.MonthsOfYear = Month.indexes_to_bitmask(manifest_hash['months'] || Month.indexes)
788802

0 commit comments

Comments
 (0)