-
Notifications
You must be signed in to change notification settings - Fork 26
(MODULES-5842) Implement Generic DSC Invoker #12
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| $script:ErrorActionPreference = 'Stop' | ||
| $script:WarningPreference = 'SilentlyContinue' | ||
|
|
||
| function new-pscredential | ||
| { | ||
| [CmdletBinding()] | ||
| param ( | ||
| [parameter(Mandatory=$true, ValueFromPipelineByPropertyName=$true)] | ||
| [string]$user, | ||
| [parameter(Mandatory=$true, ValueFromPipelineByPropertyName=$true)] | ||
| [string]$password | ||
| ) | ||
|
|
||
| $secpasswd = ConvertTo-SecureString $password -AsPlainText -Force | ||
| $credentials = New-Object System.Management.Automation.PSCredential ($user, $secpasswd) | ||
| return $credentials | ||
| } | ||
|
|
||
| $response = @{ | ||
| indesiredstate = $false | ||
| rebootrequired = $false | ||
| errormessage = '' | ||
| } | ||
|
|
||
| $invokeParams = @{ | ||
| Name = '<%= resource.parameters[:dsc_resource_name].value %>' | ||
| ModuleName = '<%= resource.parameters[:dsc_resource_module_name].value %>' | ||
| Method = '<%= dsc_invoke_method %>' | ||
| Property = <% provider.dsc_property_param.each do |p| -%> | ||
| <%= format_dsc_lite(p.value) %> | ||
| <% end -%> | ||
| } | ||
|
|
||
| try{ | ||
| $result = Invoke-DscResource @invokeParams | ||
| }catch{ | ||
| $response.errormessage = $_.Exception.Message | ||
| return ($response | ConvertTo-Json -Compress) | ||
| } | ||
|
|
||
| # keep the switch for when Test passes back changed properties | ||
| switch ($invokeParams.Method) { | ||
| 'Test' { | ||
| $response.indesiredstate = $result.InDesiredState | ||
| return ($response | ConvertTo-Json -Compress) | ||
| } | ||
| 'Set' { | ||
| $response.indesiredstate = $true | ||
| $response.rebootrequired = $result.RebootRequired | ||
| return ($response | ConvertTo-Json -Compress) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| require 'pathname' | ||
|
|
||
| Puppet::Type.newtype(:dsc) do | ||
| require Pathname.new(__FILE__).dirname + '../../' + 'puppet/type/base_dsc_lite' | ||
| require Pathname.new(__FILE__).dirname + '../../puppet_x/puppetlabs/dsc_lite/dsc_type_helpers' | ||
|
|
||
| ensurable do | ||
| newvalue(:exists?) { provider.exists? } | ||
| newvalue(:present) { provider.create } | ||
| newvalue(:absent) { provider.destroy } | ||
| defaultto { :present } | ||
| end | ||
|
|
||
| newparam(:name, :namevar => true) do | ||
| desc "Name of the declaration" | ||
| validate do |value| | ||
| if value.nil? or value.empty? | ||
| raise ArgumentError, "A non-empty #{self.name.to_s} must be specified." | ||
| end | ||
| fail("#{value} is not a valid #{self.name.to_s}") unless value =~ /^[a-zA-Z0-9\.\-\_\'\s]+$/ | ||
| end | ||
| end | ||
|
|
||
| newparam(:dsc_resource_name) do | ||
| desc "DSC Resource Name" | ||
| isrequired | ||
| validate do |value| | ||
| if value.nil? or value.empty? | ||
| raise ArgumentError, "A non-empty #{self.name.to_s} must be specified." | ||
| end | ||
| fail "#{self.name.to_s} should be a String" unless value.is_a? ::String | ||
| end | ||
| end | ||
|
|
||
| newparam(:dsc_resource_module_name) do | ||
| desc "DSC Resource Module Name" | ||
| isrequired | ||
| validate do |value| | ||
| if value.nil? or value.empty? | ||
| raise ArgumentError, "A non-empty #{self.name.to_s} must be specified." | ||
| end | ||
| fail "#{self.name.to_s} should be a String" unless value.is_a? ::String | ||
| end | ||
| end | ||
|
|
||
| newparam(:dsc_resource_properties, :array_matching => :all) do | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm - not sure if |
||
| desc "DSC Resource Properties" | ||
| isrequired | ||
| validate do |value| | ||
| if value.nil? or value.empty? | ||
| raise ArgumentError, "A non-empty #{self.name.to_s} must be specified." | ||
| end | ||
| fail "#{self.name.to_s} should be a Hash" unless value.is_a? ::Hash | ||
| end | ||
| end | ||
| end | ||
|
|
||
| Puppet::Type.type(:dsc).provide :powershell, :parent => Puppet::Type.type(:base_dsc_lite).provider(:powershell) do | ||
| confine :true => (Gem::Version.new(Facter.value(:powershell_version)) >= Gem::Version.new('5.0.10586.117')) | ||
| defaultfor :operatingsystem => :windows | ||
|
|
||
| mk_resource_methods | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| module PuppetX | ||
| module PuppetLabs | ||
| module DscLite | ||
| class PowerShellHashFormatter | ||
|
|
||
| def self.format(dsc_value) | ||
| case | ||
| when dsc_value.class.name == 'String' | ||
| self.format_string(dsc_value) | ||
| when dsc_value.class.ancestors.include?(Numeric) | ||
| self.format_number(dsc_value) | ||
| when [:true, :false].include?(dsc_value) | ||
| self.format_boolean(dsc_value) | ||
| when ['trueclass','falseclass'].include?(dsc_value.class.name.downcase) | ||
| "$#{dsc_value.to_s}" | ||
| when dsc_value.class.name == 'Array' | ||
| self.format_array(dsc_value) | ||
| when dsc_value.class.name == 'Hash' | ||
| self.format_hash(dsc_value) | ||
| else | ||
| fail "unsupported type #{dsc_value.class} of value '#{dsc_value}'" | ||
| end | ||
|
|
||
| end | ||
|
|
||
| def self.format_string(value) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had more tests coming in MODULES-6323, or at least I expect whoever implements that ticket will be adding more tests here as we'll be formatting CimInstances
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I would mark these as |
||
| "'#{escape_quotes(value)}'" | ||
| end | ||
|
|
||
| def self.format_number(value) | ||
| "#{value}" | ||
| end | ||
|
|
||
| def self.format_boolean(value) | ||
| "$#{value.to_s}" | ||
| end | ||
|
|
||
| def self.format_array(value) | ||
| output = [] | ||
| output << "@(" | ||
| value.collect do |m| | ||
| output << format(m) | ||
| end | ||
| output.join(', ') | ||
| output << ")" | ||
| end | ||
|
|
||
| def self.format_hash(value) | ||
| "@{\n" + value.collect{|k, v| format(k) + ' = ' + format(v)}.join(";\n") + "\n" + "}" | ||
| end | ||
|
|
||
| def self.escape_quotes(text) | ||
| text.gsub("'", "''") | ||
| end | ||
|
|
||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| #! /usr/bin/env ruby | ||
| require 'spec_helper' | ||
| require 'puppet/type' | ||
| require 'puppet_x/puppetlabs/dsc_lite/powershell_hash_formatter' | ||
|
|
||
| describe PuppetX::PuppetLabs::DscLite::PowerShellHashFormatter do | ||
| before(:each) do | ||
| @formatter = PuppetX::PuppetLabs::DscLite::PowerShellHashFormatter | ||
| end | ||
|
|
||
| describe "formatting ruby hash to powershell hash string" do | ||
|
|
||
| describe "when given correct hash" do | ||
|
|
||
| it "should output correct syntax with simple example" do | ||
| foo = <<-HERE | ||
| @{ | ||
| 'ensure' = 'present'; | ||
| 'name' = 'Web-WebServer' | ||
| } | ||
| HERE | ||
| result = @formatter.format({ | ||
| "ensure" => "present", | ||
| "name" => "Web-WebServer", | ||
| }) | ||
| expect(result).to eq foo.strip | ||
| end | ||
|
|
||
| it "should output correct syntax with CimInstance" do | ||
| foo = <<-HERE | ||
| @{ | ||
| 'ensure' = 'Present'; | ||
| 'bindinginfo' = @{ | ||
| 'dsc_type' = 'MSFT_xWebBindingInformation[]'; | ||
| 'dsc_properties' = @{ | ||
| 'protocol' = 'HTTP'; | ||
| 'port' = '80' | ||
| } | ||
| } | ||
| } | ||
| HERE | ||
| result = @formatter.format({ | ||
| "ensure" => "Present", | ||
| "bindinginfo" => { | ||
| "dsc_type" => "MSFT_xWebBindingInformation[]", | ||
| "dsc_properties" => { | ||
| "protocol" => "HTTP", | ||
| "port" => "80" | ||
| } | ||
| } | ||
| }) | ||
| expect(result).to eq foo.strip | ||
| end | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| require 'spec_helper' | ||
| require 'puppet/type' | ||
| require 'puppet/type/dsc' | ||
|
|
||
| describe Puppet::Type.type(:dsc) do | ||
| let(:resource) { described_class.new(:name => "dsc") } | ||
| subject { resource } | ||
|
|
||
| it { is_expected.to be_a_kind_of Puppet::Type::Dsc } | ||
|
|
||
| describe "parameter :name" do | ||
| subject { resource.parameters[:name] } | ||
|
|
||
| it { is_expected.to be_isnamevar } | ||
|
|
||
| it "should not allow nil" do | ||
| expect { | ||
| resource[:name] = nil | ||
| }.to raise_error(Puppet::Error, /Got nil value for name/) | ||
| end | ||
|
|
||
| it "should not allow empty" do | ||
| expect { | ||
| resource[:name] = '' | ||
| }.to raise_error(Puppet::ResourceError, /A non-empty name must/) | ||
| end | ||
|
|
||
| [ 'value', 'value with spaces', 'UPPER CASE', '0123456789_-', 'With.Period' ].each do |value| | ||
| it "should accept '#{value}'" do | ||
| expect { resource[:name] = value }.not_to raise_error | ||
| end | ||
| end | ||
|
|
||
| [ '*', '()', '[]', '!@' ].each do |value| | ||
| it "should reject '#{value}'" do | ||
| expect { resource[:name] = value }.to raise_error(Puppet::ResourceError, /is not a valid name/) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| describe "parameter :dsc_resource_name" do | ||
| subject { resource.parameters[:dsc_resource_name] } | ||
|
|
||
| it "should not allow nil" do | ||
| expect { | ||
| resource[:name] = nil | ||
| }.to raise_error(Puppet::Error, /Got nil value for name/) | ||
| end | ||
|
|
||
| it "should not allow empty" do | ||
| expect { | ||
| resource[:name] = '' | ||
| }.to raise_error(Puppet::ResourceError, /A non-empty name must/) | ||
| end | ||
|
|
||
| [ 'value', 'value with spaces', 'UPPER CASE', '0123456789_-', 'With.Period' ].each do |value| | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aren't DSC resource naming guidelines stricter than this? I wouldn't expect I'm having a tough time tracking down any specs / docs for the guidelines though - maybe buried somewhere in the CIM spec at https://www.dmtf.org/standards/cim/schemas?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would think trying to truly validate naming would be a game of catchup we aren't interested in pursuing as we are trying to be loosely typed here, and not get back into the business of validating parts we don't own.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at your code again, thought you had regexes in place for all of these, but I see there's only one for This is fine, though I don't expect PowerShell class names to be a moving target given they're well defined in the grammar specification. Changing that would be backward-incompatible, so I think the rules are pretty set in stone at this point. |
||
| it "should accept '#{value}'" do | ||
| expect { resource[:name] = value }.not_to raise_error | ||
| end | ||
| end | ||
|
|
||
| [ '*', '()', '[]', '!@' ].each do |value| | ||
| it "should reject '#{value}'" do | ||
| expect { resource[:name] = value }.to raise_error(Puppet::ResourceError, /is not a valid name/) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| describe "parameter :dsc_resource_module" do | ||
| subject { resource.parameters[:dsc_resource_module] } | ||
|
|
||
| it "should not allow nil" do | ||
| expect { | ||
| resource[:name] = nil | ||
| }.to raise_error(Puppet::Error, /Got nil value for name/) | ||
| end | ||
|
|
||
| it "should not allow empty" do | ||
| expect { | ||
| resource[:name] = '' | ||
| }.to raise_error(Puppet::ResourceError, /A non-empty name must/) | ||
| end | ||
|
|
||
| [ 'value', 'value with spaces', 'UPPER CASE', '0123456789_-', 'With.Period' ].each do |value| | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here for the spaces... PowerShell 3 language spec says this about Probably TMI, but ...
The important part is cannot start with Even if we don't check whether it starts with a number, I think we can disallow the whitespace.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as previous comment |
||
| it "should accept '#{value}'" do | ||
| expect { resource[:name] = value }.not_to raise_error | ||
| end | ||
| end | ||
|
|
||
| [ '*', '()', '[]', '!@' ].each do |value| | ||
| it "should reject '#{value}'" do | ||
| expect { resource[:name] = value }.to raise_error(Puppet::ResourceError, /is not a valid name/) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| describe "parameter :dsc_resource_properties" do | ||
| subject { resource.parameters[:dsc_resource_properties] } | ||
|
|
||
| it "should not allow nil" do | ||
| expect { | ||
| resource[:dsc_resource_properties] = nil | ||
| }.to raise_error(Puppet::Error, /Got nil value for dsc_resource_properties/) | ||
| end | ||
|
|
||
| it "should not allow empty" do | ||
| expect { | ||
| resource[:dsc_resource_properties] = '' | ||
| }.to raise_error(Puppet::ResourceError, /A non-empty dsc_resource_properties must be specified/) | ||
| end | ||
|
|
||
| it "requires a hash or array of hashes" do | ||
| expect { | ||
| resource[:dsc_resource_properties] = "hi" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still working this out?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was part of the exploration for supporting CimInstances, but not fully implemented past being a hash. It was left like this since we ticketed that work in MODULES-6323
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's a hash shouldn't it be? resource[:dsc_resource_properties] = { "hi" => "james" }And shouldn't it reject a bare string like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apologies - I don't know what my last comment was about here - you are rejecting a bare string 🤦♂️ |
||
| }.to raise_error(Puppet::Error, /dsc_resource_properties should be a Hash/) | ||
| expect { | ||
| resource[:dsc_resource_properties] = ["hi"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm - what does an array of hashes mean in this context? Multiple instances of the same resource? That may be complex to implement if that's the intent?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was part of the exploration for supporting CimInstances, but not fully implemented past being a hash. It was left like this since we ticketed that work in MODULES-6323
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably worth punting to next ticket / PR then |
||
| }.to raise_error(Puppet::Error, /dsc_resource_properties should be a Hash/) | ||
| end | ||
| end | ||
| end | ||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the code here the same as
format_dsc_valueabove?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the same now, however I expect it to diverge, and since you want the old way sitting next to the new way, I can't change the format_dsc method