From 86e9599a7ac25a8f8bd0f8dc2d9665ef3b697bff Mon Sep 17 00:00:00 2001
From: Justin Lambert <jlambert@eml.cc>
Date: Tue, 24 Feb 2015 07:33:44 -0700
Subject: [PATCH] add spec tests, update validations, cleanup

---
 .fixtures.yml                                 |   7 +
 .gitignore                                    |   2 +
 .travis.yml                                   |  59 +++++++
 Gemfile                                       |  27 +--
 Gemfile.lock                                  | 165 ------------------
 Rakefile                                      |  44 +++--
 files/restorecond.conf                        |   1 +
 manifests/boolean.pp                          |   4 +-
 manifests/config.pp                           |  64 +++----
 manifests/fcontext.pp                         |  45 ++---
 manifests/init.pp                             |   3 -
 manifests/metadata.json                       |  20 ---
 manifests/module.pp                           |   3 +
 manifests/package.pp                          |  29 +--
 manifests/params.pp                           |  31 +++-
 manifests/port.pp                             |  14 +-
 manifests/restorecond.pp                      |   4 +-
 manifests/restorecond/config.pp               |   8 +-
 manifests/restorecond/fragment.pp             |   1 +
 manifests/restorecond/install.pp              |   9 -
 manifests/restorecond/service.pp              |   1 +
 metadata.json                                 |   2 +-
 spec/classes/selinux_config_spec.rb           |  39 +++++
 spec/classes/selinux_package_spec.rb          |  25 +++
 .../selinux_restorecond_config_spec.rb        |  19 ++
 .../selinux_restorecond_service_spec.rb       |  17 ++
 spec/classes/selinux_restorecond_spec.rb      |  18 ++
 spec/classes/selinux_spec.rb                  |   9 +
 spec/defines/selinux_boolean_spec.rb          |  29 +++
 spec/defines/selinux_fcontext_spec.rb         |  42 +++++
 spec/defines/selinux_port_spec.rb             |  28 +++
 .../selinux_restorecond_fragment_spec.rb      |  28 +++
 spec/spec_helper.rb                           |   2 +
 33 files changed, 467 insertions(+), 332 deletions(-)
 create mode 100644 .fixtures.yml
 create mode 100644 .travis.yml
 delete mode 100644 Gemfile.lock
 delete mode 100644 manifests/metadata.json
 delete mode 100644 manifests/restorecond/install.pp
 create mode 100644 spec/classes/selinux_config_spec.rb
 create mode 100644 spec/classes/selinux_package_spec.rb
 create mode 100644 spec/classes/selinux_restorecond_config_spec.rb
 create mode 100644 spec/classes/selinux_restorecond_service_spec.rb
 create mode 100644 spec/classes/selinux_restorecond_spec.rb
 create mode 100644 spec/classes/selinux_spec.rb
 create mode 100644 spec/defines/selinux_boolean_spec.rb
 create mode 100644 spec/defines/selinux_fcontext_spec.rb
 create mode 100644 spec/defines/selinux_port_spec.rb
 create mode 100644 spec/defines/selinux_restorecond_fragment_spec.rb
 create mode 100644 spec/spec_helper.rb

diff --git a/.fixtures.yml b/.fixtures.yml
new file mode 100644
index 00000000..331b127e
--- /dev/null
+++ b/.fixtures.yml
@@ -0,0 +1,7 @@
+fixtures:
+  repositories:
+    concat: https://github.com/puppetlabs/puppetlabs-concat.git
+    stdlib: https://github.com/puppetlabs/puppetlabs-stdlib.git
+  symlinks:
+    selinux: "#{source_dir}"
+
diff --git a/.gitignore b/.gitignore
index 9f2b804d..3f413386 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,5 @@
 *.swp
 pkg/*
 vendor/bundle
+.bundle
+Gemfile.lock
diff --git a/.travis.yml b/.travis.yml
new file mode 100644
index 00000000..c253d362
--- /dev/null
+++ b/.travis.yml
@@ -0,0 +1,59 @@
+---
+language: ruby
+cache: bundler
+bundler_args: --without development
+script: "bundle exec rake validate && bundle exec rake test SPEC_OPTS='--format documentation'"
+sudo: false
+matrix:
+  fast_finish: true
+  include:
+  - rvm: 1.8.7
+    env: PUPPET_GEM_VERSION="~> 2.7.0"
+  - rvm: 1.8.7
+    env: PUPPET_GEM_VERSION="~> 3.1.0"
+  - rvm: 1.8.7
+    env: PUPPET_GEM_VERSION="~> 3.2.0"
+  - rvm: 1.8.7
+    env: PUPPET_GEM_VERSION="~> 3.3.0"
+  - rvm: 1.8.7
+    env: PUPPET_GEM_VERSION="~> 3.4.0"
+  - rvm: 1.8.7
+    env: PUPPET_GEM_VERSION="~> 3.5.0" STRICT_VARIABLES="yes"
+  - rvm: 1.8.7
+    env: PUPPET_GEM_VERSION="~> 3.6.0" STRICT_VARIABLES="yes"
+  - rvm: 1.8.7
+    env: PUPPET_GEM_VERSION="~> 3.7.0" STRICT_VARIABLES="yes"
+  - rvm: 1.9.3
+    env: PUPPET_GEM_VERSION="~> 3.1.0"
+  - rvm: 1.9.3
+    env: PUPPET_GEM_VERSION="~> 3.2.0"
+  - rvm: 1.9.3
+    env: PUPPET_GEM_VERSION="~> 3.3.0"
+  - rvm: 1.9.3
+    env: PUPPET_GEM_VERSION="~> 3.4.0"
+  - rvm: 1.9.3
+    env: PUPPET_GEM_VERSION="~> 3.5.0" STRICT_VARIABLES="yes"
+  - rvm: 1.9.3
+    env: PUPPET_GEM_VERSION="~> 3.6.0" STRICT_VARIABLES="yes"
+  - rvm: 1.9.3
+    env: PUPPET_GEM_VERSION="~> 3.7.0" STRICT_VARIABLES="yes"
+  - rvm: 2.0.0
+    env: PUPPET_GEM_VERSION="~> 3.2.0"
+  - rvm: 2.0.0
+    env: PUPPET_GEM_VERSION="~> 3.3.0"
+  - rvm: 2.0.0
+    env: PUPPET_GEM_VERSION="~> 3.4.0"
+  - rvm: 2.0.0
+    env: PUPPET_GEM_VERSION="~> 3.5.0" STRICT_VARIABLES="yes"
+  - rvm: 2.0.0
+    env: PUPPET_GEM_VERSION="~> 3.6.0" STRICT_VARIABLES="yes"
+  - rvm: 2.0.0
+    env: PUPPET_GEM_VERSION="~> 3.7.0" STRICT_VARIABLES="yes"
+  - rvm: 2.1.0
+    env: PUPPET_GEM_VERSION="~> 3.5.0" STRICT_VARIABLES="yes"
+  - rvm: 2.1.0
+    env: PUPPET_GEM_VERSION="~> 3.6.0" STRICT_VARIABLES="yes"
+  - rvm: 2.1.0
+    env: PUPPET_GEM_VERSION="~> 3.7.0" STRICT_VARIABLES="yes"
+notifications:
+  email: false
diff --git a/Gemfile b/Gemfile
index 2fdde083..880a2758 100644
--- a/Gemfile
+++ b/Gemfile
@@ -1,17 +1,24 @@
-source 'https://rubygems.org'
+source ENV['GEM_SOURCE'] || "https://rubygems.org"
 
+group :unit_tests do
+  gem 'rake',                    :require => false
+  gem 'rspec-puppet',            :require => false, :git => 'https://github.com/rodjek/rspec-puppet.git', :tag => 'v2.0.0'
+  gem 'puppetlabs_spec_helper',  :require => false
+  gem 'puppet-lint', '1.0.1',    :require => false
+  gem 'puppet-syntax',           :require => false
+  gem 'metadata-json-lint',      :require => false
+  gem 'json',                    :require => false
+end
 
-group :rake, :test do
-  gem 'puppetlabs_spec_helper', '>=0.8.2', :require => false
-  gem 'puppet-blacksmith',       :require => false
-  gem 'rspec-system-puppet',     :require => false
+group :development do
+  gem 'simplecov',   :require => false
+  gem 'guard-rake',  :require => false
 end
 
-group :rake do
-  gem 'rspec-puppet', '>=1.0.1'
-  gem 'rake',         '>=0.9.2.2'
-  gem 'puppet-lint',  '>=1.0.1'
-  gem 'rspec-system-serverspec', :require => false
+if facterversion = ENV['FACTER_GEM_VERSION']
+  gem 'facter', facterversion, :require => false
+else
+  gem 'facter', :require => false
 end
 
 if puppetversion = ENV['PUPPET_GEM_VERSION']
diff --git a/Gemfile.lock b/Gemfile.lock
deleted file mode 100644
index 1c52fe3a..00000000
--- a/Gemfile.lock
+++ /dev/null
@@ -1,165 +0,0 @@
-GEM
-  remote: https://rubygems.org/
-  specs:
-    CFPropertyList (2.3.0)
-    builder (3.2.2)
-    diff-lcs (1.2.5)
-    excon (0.43.0)
-    facter (1.7.6)
-    fission (0.5.0)
-      CFPropertyList (~> 2.2)
-    fog (1.26.0)
-      fog-atmos
-      fog-brightbox (~> 0.4)
-      fog-core (~> 1.27, >= 1.27.1)
-      fog-ecloud
-      fog-json
-      fog-profitbricks
-      fog-radosgw (>= 0.0.2)
-      fog-sakuracloud (>= 0.0.4)
-      fog-softlayer
-      fog-storm_on_demand
-      fog-terremark
-      fog-vmfusion
-      fog-voxel
-      fog-xml (~> 0.1.1)
-      ipaddress (~> 0.5)
-      nokogiri (~> 1.5, >= 1.5.11)
-    fog-atmos (0.1.0)
-      fog-core
-      fog-xml
-    fog-brightbox (0.7.1)
-      fog-core (~> 1.22)
-      fog-json
-      inflecto (~> 0.0.2)
-    fog-core (1.27.3)
-      builder
-      excon (~> 0.38)
-      formatador (~> 0.2)
-      mime-types
-      net-scp (~> 1.1)
-      net-ssh (>= 2.1.3)
-    fog-ecloud (0.0.2)
-      fog-core
-      fog-xml
-    fog-json (1.0.0)
-      multi_json (~> 1.0)
-    fog-profitbricks (0.0.1)
-      fog-core
-      fog-xml
-      nokogiri
-    fog-radosgw (0.0.3)
-      fog-core (>= 1.21.0)
-      fog-json
-      fog-xml (>= 0.0.1)
-    fog-sakuracloud (0.1.1)
-      fog-core
-      fog-json
-    fog-softlayer (0.3.27)
-      fog-core
-      fog-json
-    fog-storm_on_demand (0.1.0)
-      fog-core
-      fog-json
-    fog-terremark (0.0.3)
-      fog-core
-      fog-xml
-    fog-vmfusion (0.0.1)
-      fission
-      fog-core
-    fog-voxel (0.0.2)
-      fog-core
-      fog-xml
-    fog-xml (0.1.1)
-      fog-core
-      nokogiri (~> 1.5, >= 1.5.11)
-    formatador (0.2.5)
-    hiera (1.3.4)
-      json_pure
-    highline (1.6.21)
-    inflecto (0.0.2)
-    ipaddress (0.8.0)
-    json_pure (1.8.2)
-    kwalify (0.7.2)
-    metaclass (0.0.4)
-    mime-types (1.25.1)
-    mocha (1.1.0)
-      metaclass (~> 0.0.1)
-    multi_json (1.10.1)
-    net-scp (1.2.1)
-      net-ssh (>= 2.6.5)
-    net-ssh (2.9.2)
-    netrc (0.10.2)
-    nokogiri (1.5.11)
-    puppet (3.7.3)
-      facter (> 1.6, < 3)
-      hiera (~> 1.0)
-      json_pure
-    puppet-blacksmith (3.1.1)
-      puppet (>= 2.7.16)
-      rest-client
-    puppet-lint (1.1.0)
-    puppet-syntax (1.4.1)
-      rake
-    puppetlabs_spec_helper (0.8.2)
-      mocha
-      puppet-lint
-      puppet-syntax
-      rake
-      rspec
-      rspec-puppet
-    rake (10.4.2)
-    rbvmomi (1.8.2)
-      builder
-      nokogiri (>= 1.4.1)
-      trollop
-    rest-client (1.7.2)
-      mime-types (>= 1.16, < 3.0)
-      netrc (~> 0.7)
-    rspec (2.99.0)
-      rspec-core (~> 2.99.0)
-      rspec-expectations (~> 2.99.0)
-      rspec-mocks (~> 2.99.0)
-    rspec-core (2.99.2)
-    rspec-expectations (2.99.2)
-      diff-lcs (>= 1.1.3, < 2.0)
-    rspec-mocks (2.99.3)
-    rspec-puppet (1.0.1)
-      rspec
-    rspec-system (2.8.0)
-      fog (~> 1.18)
-      kwalify (~> 0.7.2)
-      mime-types (~> 1.16)
-      net-scp (~> 1.1)
-      net-ssh (~> 2.7)
-      nokogiri (~> 1.5.10)
-      rbvmomi (~> 1.6)
-      rspec (~> 2.14)
-      systemu (~> 2.5)
-    rspec-system-puppet (2.2.1)
-      rspec-system (~> 2.0)
-    rspec-system-serverspec (2.0.1)
-      rspec-system (~> 2.0)
-      serverspec (~> 0.0)
-      specinfra (~> 0.0)
-    serverspec (0.16.0)
-      highline
-      net-ssh
-      rspec (~> 2.13)
-      specinfra (>= 0.7.1)
-    specinfra (0.8.0)
-    systemu (2.6.4)
-    trollop (2.1.1)
-
-PLATFORMS
-  ruby
-
-DEPENDENCIES
-  puppet
-  puppet-blacksmith
-  puppet-lint (>= 1.0.1)
-  puppetlabs_spec_helper (>= 0.8.2)
-  rake (>= 0.9.2.2)
-  rspec-puppet (>= 1.0.1)
-  rspec-system-puppet
-  rspec-system-serverspec
diff --git a/Rakefile b/Rakefile
index 1878335b..67439caf 100644
--- a/Rakefile
+++ b/Rakefile
@@ -1,21 +1,39 @@
-require 'bundler'
-Bundler.require(:rake)
-require 'rake/clean'
-
-CLEAN.include('spec/fixtures/', 'doc', 'pkg')
-CLOBBER.include('.tmp', '.librarian')
-
+require 'rubygems' if RUBY_VERSION < '1.9.0'
 require 'puppetlabs_spec_helper/rake_tasks'
-require 'puppet_blacksmith/rake_tasks'
-require 'rspec-system/rake_task'
+require 'puppet-lint/tasks/puppet-lint'
+require 'puppet-syntax/tasks/puppet-syntax'
+
+begin
+  require 'puppet_blacksmith/rake_tasks'
+rescue LoadError
+end
 
-task :default => [:clean, :spec]
+exclude_paths = [
+  "pkg/**/*",
+  "vendor/**/*",
+  "spec/**/*",
+]
 
 PuppetLint.configuration.fail_on_warnings
 PuppetLint.configuration.send('relative')
 PuppetLint.configuration.send('disable_80chars')
 PuppetLint.configuration.send('disable_class_inherits_from_params_class')
-PuppetLint.configuration.send('disable_class_parameter_defaults')
 PuppetLint.configuration.send('disable_documentation')
-PuppetLint.configuration.send('disable_single_quote_string_with_variables')
-PuppetLint.configuration.ignore_paths = ["spec/**/*.pp", "pkg/**/*.pp"]
+PuppetLint.configuration.ignore_paths = exclude_paths
+#PuppetLint.configuration.send('disable_class_parameter_defaults')
+#PuppetLint.configuration.send('disable_single_quote_string_with_variables')
+PuppetSyntax.exclude_paths = exclude_paths
+
+task :default => [:test, :spec]
+
+task :metadata do
+  sh "metadata-json-lint metadata.json"
+end
+
+desc "Run syntax, lint, and spec tests."
+task :test => [
+  :syntax,
+  :lint,
+  :metadata,
+  :spec,
+]
diff --git a/files/restorecond.conf b/files/restorecond.conf
index 58b723a2..e0517b13 100644
--- a/files/restorecond.conf
+++ b/files/restorecond.conf
@@ -1,3 +1,4 @@
+# File Managed by Puppet
 /etc/services
 /etc/resolv.conf
 /etc/samba/secrets.tdb
diff --git a/manifests/boolean.pp b/manifests/boolean.pp
index 24486ff6..5a4c17d6 100644
--- a/manifests/boolean.pp
+++ b/manifests/boolean.pp
@@ -26,9 +26,11 @@
 #
 
 define selinux::boolean (
-  $ensure = 'undef'
+  $ensure = true
 ) {
 
+  include selinux
+
   Exec {
     path => '/bin:/sbin:/usr/bin:/usr/sbin',
   }
diff --git a/manifests/config.pp b/manifests/config.pp
index 0a689004..e3cc8d24 100644
--- a/manifests/config.pp
+++ b/manifests/config.pp
@@ -18,54 +18,42 @@
 class selinux::config (
   $mode = $::selinux::mode,
 ) {
-  Exec {
-    path => '/bin:/sbin:/usr/bin:/usr/sbin',
+
+  if $caller_module_name != $module_name {
+    fail("Use of private class ${name} by ${caller_module_name}")
   }
 
+  # Validations
+  validate_re($mode, ['^enforcing$', '^permissive$', '^disabled'], "Valid modes are enforcing, permissive, and disabled.  Received: ${mode}")
+
   file { $selinux::params::sx_mod_dir:
     ensure => directory,
   }
 
-  # Check to see if the mode set is valid.
-  if $mode == 'enforcing' or $mode == 'permissive' or $mode == 'disabled' {
-    case $::operatingsystemrelease {
-      # Change command based on OS release.
-      # RHEL <= 5 do not support --follow-symlinks with sed
-      # ref: @lboynton: http://git.io/QvJ9ww
-      /^5/: {
-        $selinux_set_command = "sed -i \"s@^\\(SELINUX=\\).*@\\1${mode}@\" /etc/selinux/config"
-      }
-      default: {
-        $selinux_set_command = "sed -i --follow-symlinks \"s@^\\(SELINUX=\\).*@\\1${mode}@\" /etc/selinux/config"
+  file_line { "set-selinux-config-to-${mode}":
+    path  => '/etc/selinux/config',
+    line  => "SELINUX=${mode}",
+    match => '^SELINUX=\w+',
+  }
+
+  case $mode {
+    permissive, disabled: {
+      $sestatus = '0'
+      if $mode == 'disabled' and $::selinux_current_mode == 'permissive' {
+        notice('A reboot is required to fully disable SELinux. SELinux will operate in Permissive mode until a reboot')
       }
     }
-
-    exec { "set-selinux-config-to-${mode}":
-      command => $selinux_set_command,
-      unless  => "grep -q \"SELINUX=${mode}\" /etc/selinux/config",
+    enforcing: {
+      $sestatus = '1'
     }
-
-    case $mode {
-      permissive,disabled: {
-        $sestatus = '0'
-        if $mode == 'disabled' and $::selinux_current_mode == 'permissive' {
-          notice('A reboot is required to fully disable SELinux. SELinux will operate in Permissive mode until a reboot')
-        }
-      }
-      enforcing: {
-        $sestatus = '1'
-      }
-      default : {
-        fail('You must specify a mode (enforced, permissive, or disabled) for selinux operation')
-      }
+    default : {
+      fail('You must specify a mode (enforced, permissive, or disabled) for selinux operation')
     }
+  }
 
-    exec { "change-selinux-status-to-${mode}":
-      command => "setenforce ${sestatus}",
-      unless  => "getenforce | grep -qi \"${mode}\\|disabled\"",
-      path    => '/bin:/usr/bin:/usr/sbin',
-    }
-  } else {
-    fail("Invalid mode specified for SELinux: ${mode}")
+  exec { "change-selinux-status-to-${mode}":
+    command => "setenforce ${sestatus}",
+    unless  => "getenforce | grep -qi \"${mode}\\|disabled\"",
+    path    => '/bin:/usr/bin:/usr/sbin',
   }
 }
diff --git a/manifests/fcontext.pp b/manifests/fcontext.pp
index dcabafa5..a0b7de07 100644
--- a/manifests/fcontext.pp
+++ b/manifests/fcontext.pp
@@ -40,7 +40,7 @@
 #
 # Sample Usage:
 #
-# FOR SUBSTITUTING TARGET PATH WITH SOURCEPATH: 
+# FOR SUBSTITUTING TARGET PATH WITH SOURCEPATH:
 # selinux::fcontext{'set-postfix-instance1-spool':
 #   equals      => true,
 #   pathname    => '/var/spool/postfix-instance1',
@@ -63,50 +63,40 @@
 # }
 #
 define selinux::fcontext (
-  $destination = '',
+  $pathname,
+  $destination = undef,
   $context     = '',
-  $pathname    = '',
   $filetype    = false,
-  $filemode    = '',
+  $filemode    = undef,
   $equals      = false,
-  $policy      = 'targeted',
 ) {
-  Exec {
-    path => '/bin:/sbin:/usr/bin:/usr/sbin',
-  }
-  if $pathname == '' {
-    fail('pathname must not be empty')
-  }
 
-  if ( $equals == false ) and ( $pathname == '' ) {
-    fail('pathname must not be empty')
-  }
+  include selinux
 
-  if ( $equals == true ) and ( $destination == '' ) {
-    fail('destination must be set if equals is true')
+  validate_absolute_path($pathname)
+  validate_bool($filetype, $equals)
+
+  if $equals {
+    validate_absolute_path($destination)
   }
 
-  if ( $filetype == true ) and ( $filemode == '' ) {
-    fail('file mode must not be empty')
+  if $equals and $filetype {
+    fail('Resource cannot contain both "equals" and "filetype" options')
   }
-  elsif ( $filetype == true ) and ( $filemode !~ /(a|f|d|c|b|s|l|p)/ ) {
+
+  if $filetype and $filemode !~ /(a|f|d|c|b|s|l|p)/ {
     fail('file mode must be one of: a,f,d,c,b,s,l,p - see "man semanage-fcontext"')
   }
 
-  if ( $equals == true ) and ( $filetype == true) {
-    fail('Resource cannot contain both "equals" and "filetype" options')
-  }
-  elsif ( $equals == true ) {
+  if $equals {
     $resource_name = "add_${destination}_${pathname}"
     $command       = "semanage fcontext -a -e \"${destination}\" \"${pathname}\""
     $unless        = "semanage fcontext -l | grep -E \"^${pathname} = ${destination}$\""
-  }
-  elsif ( $equals == false ) and ( $filetype == true ) {
+  } elsif $filetype {
     $resource_name = "add_${context}_${pathname}_type_${filemode}"
     $command       = "semanage fcontext -a -f ${filemode} -t ${context} \"${pathname}\""
     $unless        = "semanage fcontext -l | grep -E \"^${pathname}.*:${context}:\""
-  }
-  elsif ( $equals == false ) and ( $filetype == false ) {
+  } else {
     $resource_name = "add_${context}_${pathname}"
     $command       = "semanage fcontext -a -t ${context} \"${pathname}\""
     $unless        = "semanage fcontext -l | grep -E \"^${pathname}.*:${context}:\""
@@ -115,6 +105,7 @@
   exec { $resource_name:
     command => $command,
     unless  => $unless,
+    path    => '/bin:/sbin:/usr/bin:/usr/sbin',
     require => Class['selinux::package'],
   }
 }
diff --git a/manifests/init.pp b/manifests/init.pp
index ec1acdad..f2b590bc 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -18,11 +18,8 @@
 #
 class selinux (
   $mode = $::selinux::params::mode,
-  $package_ensure = $::selinux::params::package_ensure,
 ) inherits selinux::params {
 
-  include stdlib
-
   class { 'selinux::package': } ->
   class { 'selinux::config': }
 }
diff --git a/manifests/metadata.json b/manifests/metadata.json
deleted file mode 100644
index 77f128c2..00000000
--- a/manifests/metadata.json
+++ /dev/null
@@ -1,20 +0,0 @@
-{
-  "name": "jfryman/selinux",
-  "version": "0.1.1",
-  "author": "jfryman",
-  "summary": "This class manages SELinux on RHEL based systems",
-  "license": "Apache 2.0",
-  "source": "https://github.com/jfryman/puppet-selinux",
-  "project_page": "https://github.com/jfryman/puppet-selinux",
-  "issues_url": "https://github.com/jfryman/puppet-selinux/issues",
-  "dependencies": [
-    {
-      "name": "puppetlabs-stdlib",
-      "version_range": ">= 4.0.0"
-    },
-    {
-      "name": "puppetlabs-concat",
-      "version_range": ">= 1.0.0"
-    }
-  ]
-}
diff --git a/manifests/module.pp b/manifests/module.pp
index d3681c4a..8a1c6cb9 100644
--- a/manifests/module.pp
+++ b/manifests/module.pp
@@ -32,6 +32,9 @@
   $use_makefile   = false,
   $makefile       = '/usr/share/selinux/devel/Makefile',
 ) {
+
+  include selinux
+
   # Set Resource Defaults
   File {
     owner => 'root',
diff --git a/manifests/package.pp b/manifests/package.pp
index 93e8eead..f0af5454 100644
--- a/manifests/package.pp
+++ b/manifests/package.pp
@@ -14,26 +14,13 @@
 #
 # This class file is not called directly
 class selinux::package {
-  case $::operatingsystem {
-    CentOS,Fedora,RHEL,RedHat,Scientific: {
-      case $::operatingsystemrelease {
-        /^5.+$/: {
-          package { 'policycoreutils':
-            ensure => $selinux::params::package_ensure,
-          }
-        }
-        /^(6|7).+$/: {
-          package { 'policycoreutils-python':
-            ensure => $selinux::params::package_ensure,
-          }
-        }
-        default: {
-          # We only deal with RHEL (or deriviative) 5, 6 or 7.
-        }
-      }
-    }
-    default: {
-      # Nothing to do, only manage SELinux on OS's defined above
-    }
+
+  if $caller_module_name != $module_name {
+    fail("Use of private class ${name} by ${caller_module_name}")
   }
+
+  package { $::selinux::params::package_name:
+    ensure => installed,
+  }
+
 }
diff --git a/manifests/params.pp b/manifests/params.pp
index 2b3715d7..713c3ad2 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -9,18 +9,35 @@
 class selinux::params {
   $sx_mod_dir   = '/usr/share/selinux'
   $mode         = 'disabled'
-  $package_ensure = present
 
-  $sx_fs_mount  = $::osfamily ? {
-    'RedHat' => $::operatingsystemmajrelease ? {
-      '7'           => '/sys/fs/selinux',
-      default       => '/selinux',
-    },
-    default         => '/selinux',
+  case $::osfamily {
+    'RedHat': {
+      case $::operatingsystemmajrelease {
+        '7': {
+          $sx_fs_mount = '/sys/fs/selinux'
+          $package_name = 'policycoreutils-python'
+        }
+        '6': {
+          $sx_fs_mount = '/selinux'
+          $package_name = 'policycoreutils-python'
+        }
+        '5': {
+          $sx_fs_mount = '/selinux'
+          $package_name = 'policycoreutils'
+        }
+        default: {
+          fail("${::osfamily}-${::operatingsystemmajrelease} is not supported")
+        }
+      }
+    }
+    default: {
+      fail("${::osfamily} is not supported")
+    }
   }
 
   $restorecond_config_file       = '/etc/selinux/restorecond.conf'
   $restorecond_config_file_mode  = '0644'
   $restorecond_config_file_owner = 'root'
   $restorecond_config_file_group = 'root'
+
 }
diff --git a/manifests/port.pp b/manifests/port.pp
index d5096cb3..dd6da6aa 100644
--- a/manifests/port.pp
+++ b/manifests/port.pp
@@ -36,20 +36,15 @@
 #  }
 #
 define selinux::port (
-  $context  = undef,
+  $context,
+  $port,
   $protocol = undef,
-  $port     = undef,
-  $policy   = 'targeted'
 ) {
-  Exec {
-    path => '/bin:/sbin:/usr/bin:/usr/sbin',
-  }
 
-  if ( $context == undef ) or ( $port == undef ) {
-    fail('context and port must not be empty')
-  }
+  include selinux
 
   if $protocol {
+    validate_re($protocol, ['^tcp6?$', '^udp6?$'])
     $protocol_switch="-p ${protocol} "
   } else {
     $protocol_switch=''
@@ -58,6 +53,7 @@
   exec { "add_${context}_${port}":
     command => "semanage port -a -t ${context} ${protocol_switch}${port}",
     unless  => "semanage port -l|grep \"^${context}.*${protocol}.*${port}\"",
+    path    => '/bin:/sbin:/usr/bin:/usr/sbin',
     require => Class['selinux::package']
   }
 }
diff --git a/manifests/restorecond.pp b/manifests/restorecond.pp
index e730eea5..64f70ce5 100644
--- a/manifests/restorecond.pp
+++ b/manifests/restorecond.pp
@@ -7,7 +7,9 @@
   $config_file_owner = $selinux::params::restorecond_config_file_owner,
   $config_file_group = $selinux::params::restorecond_config_file_group,
 ) inherits selinux::params {
-  class{'selinux::restorecond::install':} ->
+
+  include selinux
+  Class['selinux'] ->
   class{'selinux::restorecond::config':} ~>
   class{'selinux::restorecond::service':}
 }
diff --git a/manifests/restorecond/config.pp b/manifests/restorecond/config.pp
index 618778a7..02c5dfee 100644
--- a/manifests/restorecond/config.pp
+++ b/manifests/restorecond/config.pp
@@ -11,15 +11,9 @@
     notify => Service['restorecond'],
   }
 
-  concat::fragment {'restorecond_config_header':
-    target  => $selinux::restorecond::config_file,
-    content => "# File Managed by Puppet\n",
-    order   => '01'
-  }
-
   concat::fragment {'restorecond_config_default':
     target => $selinux::restorecond::config_file,
     source => 'puppet:///modules/selinux/restorecond.conf',
-    order  => '05'
+    order  => '01'
   }
 }
diff --git a/manifests/restorecond/fragment.pp b/manifests/restorecond/fragment.pp
index 6f722583..58e39fba 100644
--- a/manifests/restorecond/fragment.pp
+++ b/manifests/restorecond/fragment.pp
@@ -6,6 +6,7 @@
   $source = undef,
   $order = '10'
 ) {
+
   if !defined(Class['selinux::restorecond']) {
     fail('You must include the restorecond base class before using any restorecond defined resources')
   }
diff --git a/manifests/restorecond/install.pp b/manifests/restorecond/install.pp
deleted file mode 100644
index 7fd6935f..00000000
--- a/manifests/restorecond/install.pp
+++ /dev/null
@@ -1,9 +0,0 @@
-#
-# Class selinux::restorecond::install
-#
-class selinux::restorecond::install {
-  package {
-    'policycoreutils':
-      ensure => present;
-  }
-}
diff --git a/manifests/restorecond/service.pp b/manifests/restorecond/service.pp
index 6c3c54a9..3a2f145e 100644
--- a/manifests/restorecond/service.pp
+++ b/manifests/restorecond/service.pp
@@ -1,5 +1,6 @@
 # manages restorecond service
 class selinux::restorecond::service {
+
   service{'restorecond':
     ensure => running,
     enable => true,
diff --git a/metadata.json b/metadata.json
index 38b59300..e52fdeb9 100644
--- a/metadata.json
+++ b/metadata.json
@@ -3,7 +3,7 @@
 "version": "0.2.2",
 "author": "jfryman",
 "summary": "This class manages SELinux on RHEL based systems",
-"license": "Apache 2.0",
+"license": "Apache-2.0",
 "source": "https://github.com/jfryman/puppet-selinux",
 "project_page": "https://github.com/jfryman/puppet-selinux",
 "issues_url": "https://github.com/jfryman/puppet-selinux/issues",
diff --git a/spec/classes/selinux_config_spec.rb b/spec/classes/selinux_config_spec.rb
new file mode 100644
index 00000000..d44f87a0
--- /dev/null
+++ b/spec/classes/selinux_config_spec.rb
@@ -0,0 +1,39 @@
+require 'spec_helper'
+
+describe 'selinux' do
+  let(:facts) { { :osfamily => 'RedHat', :operatingsystemmajrelease => '7', :selinux_current_mode => 'enforcing' } }
+
+  context 'config' do
+
+    context 'invalid mode' do
+      let(:params) { { :mode => 'invalid' } }
+      it { expect { should create_class('selinux') }.to raise_error(/Valid modes are enforcing, permissive, and disabled.  Received: invalid/) }
+    end
+
+    context 'enforcing' do
+      let(:params) { { :mode => 'enforcing' } }
+
+      it { should contain_file('/usr/share/selinux').with(:ensure => 'directory') }
+      it { should contain_file_line('set-selinux-config-to-enforcing').with(:line => 'SELINUX=enforcing') }
+      it { should contain_exec('change-selinux-status-to-enforcing').with(:command => 'setenforce 1') }
+    end
+
+    context 'permissive' do
+      let(:params) { { :mode => 'permissive' } }
+
+      it { should contain_file('/usr/share/selinux').with(:ensure => 'directory') }
+      it { should contain_file_line('set-selinux-config-to-permissive').with(:line => 'SELINUX=permissive') }
+      it { should contain_exec('change-selinux-status-to-permissive').with(:command => 'setenforce 0') }
+    end
+
+    context 'disabled' do
+      let(:params) { { :mode => 'disabled' } }
+
+      it { should contain_file('/usr/share/selinux').with(:ensure => 'directory') }
+      it { should contain_file_line('set-selinux-config-to-disabled').with(:line => 'SELINUX=disabled') }
+      it { should contain_exec('change-selinux-status-to-disabled').with(:command => 'setenforce 0') }
+    end
+
+  end
+
+end
diff --git a/spec/classes/selinux_package_spec.rb b/spec/classes/selinux_package_spec.rb
new file mode 100644
index 00000000..a93149a9
--- /dev/null
+++ b/spec/classes/selinux_package_spec.rb
@@ -0,0 +1,25 @@
+require 'spec_helper'
+
+describe 'selinux' do
+
+  context 'package' do
+
+    context 'on RedHat 5 based OSes' do
+      let(:facts) { { :osfamily => 'RedHat', :operatingsystemmajrelease => '5', :selinux_current_mode => 'enforcing' } }
+
+      it { should contain_package('policycoreutils').with(:ensure => 'installed') }
+    end
+
+    [ '6', '7' ].each do |majrelease|
+      context "On RedHat #{majrelease} based OSes" do
+        let(:facts) { { :osfamily => 'RedHat', :operatingsystemmajrelease => majrelease, :selinux_current_mode => 'enforcing' } }
+
+        it { should contain_package('policycoreutils-python').with(:ensure => 'installed') }
+      end
+    end
+
+
+  end
+
+end
+
diff --git a/spec/classes/selinux_restorecond_config_spec.rb b/spec/classes/selinux_restorecond_config_spec.rb
new file mode 100644
index 00000000..4f73d14e
--- /dev/null
+++ b/spec/classes/selinux_restorecond_config_spec.rb
@@ -0,0 +1,19 @@
+require 'spec_helper'
+
+describe 'selinux::restorecond' do
+  let(:facts) { {
+    :osfamily => 'RedHat',
+    :operatingsystemmajrelease => '7',
+    :selinux_current_mode => 'enforcing',
+    # concat facts
+    :concat_basedir => '/tmp',
+    :id => 0,
+    :is_pe => false,
+    :path => '/tmp',
+  } }
+
+  it { should contain_concat('/etc/selinux/restorecond.conf') }
+  it { should contain_concat__fragment('restorecond_config_default') }
+
+end
+
diff --git a/spec/classes/selinux_restorecond_service_spec.rb b/spec/classes/selinux_restorecond_service_spec.rb
new file mode 100644
index 00000000..c028fa82
--- /dev/null
+++ b/spec/classes/selinux_restorecond_service_spec.rb
@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe 'selinux::restorecond' do
+  let(:facts) { {
+    :osfamily => 'RedHat',
+    :operatingsystemmajrelease => '7',
+    :selinux_current_mode => 'enforcing',
+    # concat facts
+    :concat_basedir => '/tmp',
+    :id => 0,
+    :is_pe => false,
+    :path => '/tmp',
+  } }
+
+  it { should contain_service('restorecond') }
+
+end
diff --git a/spec/classes/selinux_restorecond_spec.rb b/spec/classes/selinux_restorecond_spec.rb
new file mode 100644
index 00000000..de0b5a9b
--- /dev/null
+++ b/spec/classes/selinux_restorecond_spec.rb
@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+describe 'selinux::restorecond' do
+  let(:facts) { {
+    :osfamily => 'RedHat',
+    :operatingsystemmajrelease => '7',
+    :selinux_current_mode => 'enforcing',
+    # concat facts
+    :concat_basedir => '/tmp',
+    :id => 0,
+    :is_pe => false,
+    :path => '/tmp',
+  } }
+
+  it { should contain_class('selinux::restorecond::config') }
+  it { should contain_class('selinux::restorecond::service') }
+
+end
diff --git a/spec/classes/selinux_spec.rb b/spec/classes/selinux_spec.rb
new file mode 100644
index 00000000..698a71e1
--- /dev/null
+++ b/spec/classes/selinux_spec.rb
@@ -0,0 +1,9 @@
+require 'spec_helper'
+
+describe 'selinux' do
+  let(:facts) { { :osfamily => 'RedHat', :operatingsystemmajrelease => '7', :selinux_current_mode => 'enforcing' } }
+
+  it { should contain_class('selinux::package') }
+  it { should contain_class('selinux::config') }
+
+end
diff --git a/spec/defines/selinux_boolean_spec.rb b/spec/defines/selinux_boolean_spec.rb
new file mode 100644
index 00000000..0757e3a7
--- /dev/null
+++ b/spec/defines/selinux_boolean_spec.rb
@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe 'selinux::boolean' do
+  let(:title) { 'mybool' }
+  let(:facts) { {
+    :osfamily => 'RedHat',
+    :operatingsystemmajrelease => '7',
+    :selinux_current_mode => 'enforcing',
+  } }
+
+  context 'default' do
+    it { should contain_exec("setsebool -P 'mybool' true")}
+  end
+
+  ['on', true].each do |value|
+    context value do
+      let(:params) { { :ensure => value } }
+      it { should contain_exec("setsebool -P 'mybool' true")}
+    end
+  end
+
+  ['off', false].each do |value|
+    context value do
+      let(:params) { { :ensure => value } }
+      it { should contain_exec("setsebool -P 'mybool' false")}
+    end
+  end
+
+end
diff --git a/spec/defines/selinux_fcontext_spec.rb b/spec/defines/selinux_fcontext_spec.rb
new file mode 100644
index 00000000..36876755
--- /dev/null
+++ b/spec/defines/selinux_fcontext_spec.rb
@@ -0,0 +1,42 @@
+require 'spec_helper'
+
+describe 'selinux::fcontext' do
+  let(:title) { 'myfile' }
+  let(:facts) { {
+    :osfamily => 'RedHat',
+    :operatingsystemmajrelease => '7',
+    :selinux_current_mode => 'enforcing',
+  } }
+
+  context 'invalid pathname' do
+    it { expect { is_expected.to compile }.to raise_error }
+  end
+
+  context 'equal requires destination' do
+    it { expect { is_expected.to compile }.to raise_error }
+  end
+
+  context 'invalid filetype' do
+    it { expect { is_expected.to compile }.to raise_error }
+  end
+
+  context 'equals and filetype' do
+    it { expect { is_expected.to compile }.to raise_error }
+  end
+
+  context 'substituting fcontext' do
+    let(:params) { { :pathname => '/tmp/file1', :equals => true, :destination => '/tmp/file2' } }
+    it { should contain_exec('add_/tmp/file2_/tmp/file1').with(:command => 'semanage fcontext -a -e "/tmp/file2" "/tmp/file1"') }
+  end
+
+  context 'set filemode and context' do
+    let(:params) { { :pathname => '/tmp/file1', :filetype => true, :filemode => 'a', :context => 'user_home_dir_t' } }
+    it { should contain_exec('add_user_home_dir_t_/tmp/file1_type_a').with(:command => 'semanage fcontext -a -f a -t user_home_dir_t "/tmp/file1"') }
+  end
+
+  context 'set context' do
+    let(:params) { { :pathname => '/tmp/file1', :context => 'user_home_dir_t' } }
+    it { should contain_exec('add_user_home_dir_t_/tmp/file1').with(:command => 'semanage fcontext -a -t user_home_dir_t "/tmp/file1"') }
+  end
+
+end
diff --git a/spec/defines/selinux_port_spec.rb b/spec/defines/selinux_port_spec.rb
new file mode 100644
index 00000000..1cd81662
--- /dev/null
+++ b/spec/defines/selinux_port_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+
+describe 'selinux::port' do
+  let(:title) { 'myapp' }
+  let(:facts) { {
+    :osfamily => 'RedHat',
+    :operatingsystemmajrelease => '7',
+    :selinux_current_mode => 'enforcing',
+  } }
+
+  ['tcp', 'udp', 'tcp6', 'udp6'].each do |protocol|
+    context "valid protocol #{protocol}" do
+      let(:params) { { :context => 'http_port_t', :port => 8080, :protocol => protocol } }
+      it { should contain_exec('add_http_port_t_8080').with(:command => "semanage port -a -t http_port_t -p #{protocol} 8080") }
+    end
+  end
+
+  context 'invalid protocol' do
+    let(:params) { { :context => 'http_port_t', :port => 8080, :protocol => 'bad' } }
+    it { expect { is_expected.to compile }.to raise_error }
+  end
+
+  context 'no protocol' do
+    let(:params) { { :context => 'http_port_t', :port => 8080 } }
+    it { should contain_exec('add_http_port_t_8080').with(:command => 'semanage port -a -t http_port_t 8080') }
+  end
+
+end
diff --git a/spec/defines/selinux_restorecond_fragment_spec.rb b/spec/defines/selinux_restorecond_fragment_spec.rb
new file mode 100644
index 00000000..d66af878
--- /dev/null
+++ b/spec/defines/selinux_restorecond_fragment_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+
+describe 'selinux::restorecond::fragment' do
+  let(:pre_condition) { 'class { "selinux::restorecond": }' }
+  let(:title) { 'cond' }
+  let(:facts) { {
+    :osfamily => 'RedHat',
+    :operatingsystemmajrelease => '7',
+    :selinux_current_mode => 'enforcing',
+    # concat facts
+    :concat_basedir => '/tmp',
+    :id => 0,
+    :is_pe => false,
+    :path => '/tmp',
+  } }
+
+  context 'source' do
+    let(:params) { { :source => 'puppet:///data/cond.txt' } }
+    it { should contain_concat__fragment('restorecond_conf_cond').with(:source => 'puppet:///data/cond.txt', :order => 10 ) }
+  end
+
+  context 'content and order' do
+    let(:params) { { :content => '/etc/myapp', :order => 20 } }
+    it { should contain_concat__fragment('restorecond_conf_cond').with(:content => '/etc/myapp', :order => 20 ) }
+  end
+
+end
+
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
new file mode 100644
index 00000000..ecdad117
--- /dev/null
+++ b/spec/spec_helper.rb
@@ -0,0 +1,2 @@
+require 'rubygems' if RUBY_VERSION < '1.9.0'
+require 'puppetlabs_spec_helper/module_spec_helper'