voxpupuli / puppet-windowsfeature

Library that uses ServerAdministration api that comes with Windows Server 2008 and Windows Server 2012 to add / remove windows features
https://forge.puppet.com/puppet/windowsfeature
MIT License
30 stars 49 forks source link

v2.0.0 Does Not Work with Default PowerShell (v2) on Windows 2008R2 #57

Closed ferventcoder closed 8 years ago

ferventcoder commented 8 years ago

https://github.com/voxpupuli/puppet-windowsfeature/blob/0988ebfc64c0281498aeebb8c3ee2b3e3348fa9f/lib/puppet/provider/windowsfeature/default.rb#L18

ConvertTo-JSON - https://technet.microsoft.com/en-us/library/hh849922(v=wps.620).aspx

This cmdlet is introduced in Windows PowerShell 3.0.

ferventcoder commented 8 years ago

https://github.com/voxpupuli/puppet-windowsfeature/commit/ff0725310681f1063464c015d1e155709144b0b2 is the commit that introduced this.

bbriggs commented 8 years ago

We use this cmdlet in IIS as well, where it appears to be the only thing that is blocking on us supporting 2008.

Is there a way to achieve the functionality of ConvertTo-JSON without using that cmdlet itself

ferventcoder commented 8 years ago

We did something for PowerShell v2. I don't remember if we rolled our own, but short of that, that is what you will end up doing if you want similar functionality. Provide a PowerShell polyfill.

@jpogran @IriStyle @glennsarti ^^

glennsarti commented 8 years ago

@ferventcoder We used XML and Base64 encoding

petems commented 8 years ago

Isn't 2008R2 EOL?

glennsarti commented 8 years ago

Nope. It's in extended support. EOL is 14th Jan 2020 https://support.microsoft.com/en-us/lifecycle?p1=14134

petems commented 8 years ago

Hah, I looked on that exact page and missed the extended column 😓

Iristyle commented 8 years ago

You have a few options:

dassump commented 8 years ago

I have many windows server 2008r2 with Default PowerShell v2. I am in doubt about using the old version 1.0.1 of this module or return to the puppetlabs-dism.

matthewrstone commented 8 years ago

I'm working on getting this fixed by using CSV if it detects 2008/r2...ahhhh legacy.

ferventcoder commented 8 years ago

@matthewrstone I think I would detect version of PowerShell, more deterministic. We do that like this:

if ($PSVersionTable.PSVersion -le [Version]'2.0'){ }

https://github.com/puppetlabs/puppetlabs-powershell/blob/6d5352d85a53a13e9e4eaa6d41e903842106ea03/lib/puppet_x/templates/init_ps.ps1.erb#L400

matthewrstone commented 8 years ago

Alright...so I'm looking at something like this right now that seems to do the trick. Still needs a little cleanup, though...

def self.ps_version
    response = ps("$PSVersionTable.PSVersion -le [Version]'2.0'")
    return response
  end

  def self.instances
    crusty = ps_version
    if crusty
      features = Array.new
      get_features = ps("import-module servermanager;get-windowsfeature|select name,installed|convertto-csv")
      csv = CSV.new(get_features).to_a     
      csv.each do | name,installed |
        if installed == 'True'
          status = true
        elsif installed == 'False'
          status = false
        end
        feature_hash = {
          "Name" => name,
          "Installed" => status
        } 
        features.push(feature_hash)
      end
    else
      get_features = "Get-WindowsFeature|ConvertTo-Json"
      features = JSON.parse(ps(get_features))
    end
    features.map do |feature|
      name = feature['Name'].downcase
      installed = feature['Installed']
      if installed == true
        currentstate = :present
      elsif installed == false
        currentstate = :absent
      end
      new(:name => name, :ensure => currentstate)
    end
  end
glennsarti commented 8 years ago

Do you really need the JSON code branch at all? CSV is supported in PS3,4,5 ?

matthewrstone commented 8 years ago

I think it can just go. It was originally there with an idea to pull more info from the feature other than name and installstate, but that can be handled with a little bit of less hacky CSV parsing and there is less than zero demand for it currently, so no problems here.

matthewrstone commented 8 years ago

Alright, PR #58 is up and should resolve this.