Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(GH-55) Handle intentionally empty arrays #56

Merged
merged 4 commits into from
Nov 17, 2020
Merged

(GH-55) Handle intentionally empty arrays #56

merged 4 commits into from
Nov 17, 2020

Conversation

michaeltlombardi
Copy link
Contributor

Prior to this commit the DSC base provider would occasionally need to send an empty array as a defined property to Invoke-DscResource. However, the CIM instance on the other end may not be able to appropriately cast from an empty array to, say a [String[]] type; this would cause the run to fail. Furthermore, the provider did not have a way to distinguish between an absent setting and an empty one. Ruby does not treat empty arrays and nil as the same, even though PowerShell can/does.

This commit adds a special handler for munging the data returned from a get invocation, ensuring that if a property's value is nil AND the property was used for the query AND the resource expects an array, THEN it will set the result to the manifest value or an empty array.

It also strongly types any empty arrays being sent
to Invoke-DscResource, as this prevents the casting problem by making the array type casting explicit instead of merely implicit.

Prior to this commit the DSC base provider would
occasionally need to send an empty array as a
defined property to Invoke-DscResource. However,
the CIM instance on the other end may not be able
to appropriately cast from an empty array to, say
a [String[]] type; this would cause the run to
fail. Furthermore, the provider did not have a way
to distinguish between an *absent* setting and an
*empty* one. Ruby does not treat empty arrays and
nil as the same, even though PowerShell can/does.

This commit adds a special handler for munging the
data returned from a get invocation, ensuring that
if a propertys value is nil AND the property was
used for the query AND the resource expects an
array, THEN it will set the result to the manifest
value or an empty array.

It also strongly types any empty arrays being sent
to Invoke-DscResource, as this prevents the casting
problem by making the array type casting  explicit
instead of merely implicit.

- Resolves #55
@michaeltlombardi michaeltlombardi requested a review from a team as a code owner November 12, 2020 21:47
@michaeltlombardi
Copy link
Contributor Author

Lots of rubocop changes since last PR, will address those in an additional maint commit.

@@ -283,9 +285,9 @@ def should_to_resource(should, context, dsc_invoke_method)
# During a Puppet agent run, the code lives in the cache so we can use the file expansion to discover the correct folder.
root_module_path = $LOAD_PATH.select { |path| path.match?(%r{#{resource[:dscmeta_module_name].downcase}/lib}) }.first
resource[:vendored_modules_path] = if root_module_path.nil?
File.expand_path(Pathname.new(__FILE__).dirname + '../../../' + 'puppet_x/dsc_resources')
File.expand_path(Pathname.new(__FILE__).dirname + '../../../' + 'puppet_x/dsc_resources') # rubocop:disable Style/StringConcatenation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubocop: Hey man! You can't do that
Lombardi: STFU
Me: LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt like in this case, with going up-level, it was more readable/understandable with the + instead of interpolation - looks like steps to get to what you want, imo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey man, you had me at rubocop:disable.

empty_array_parameters = resource[:parameters].select { |_k, v| v[:value].empty? }
empty_array_parameters.each do |name, properties|
param_block_name = name.to_s.gsub(/^dsc_/, '')
params_block = params_block.gsub("#{param_block_name} = @()", "#{param_block_name} = [#{properties[:mof_type]}]@()")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JFC. I'm both glad you were able to figure this out, and saddened that you had to. I like it though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an extremely unclever hack but it did work in this case.

@RandomNoun7 RandomNoun7 merged commit a65f6fa into puppetlabs:main Nov 17, 2020
@michaeltlombardi michaeltlombardi deleted the gh-55/main/handle-empty-arrays branch November 17, 2020 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty Arrays cause casting errors for CIM instances
2 participants