Updating #343 to use inline policies for roles, expanding test coverage

lib/chef/provider/aws_iam_instance_profile.rb

@@ -9,11 +9,14 @@ def action_create
     update_attached_role(iam_instance_profile)
   end
 
+
   protected
 
   def detach_role(iam_instance_profile)
     iam_instance_profile.roles.each do |r|
-      iam_instance_profile.remove_role(role_name: r.name)
+      converge_by "detaching role #{r.name} from instance profile #{new_resource.name}" do
+        iam_instance_profile.remove_role(role_name: r.name)
+      end
     end
   end
 
@@ -22,9 +25,9 @@ def update_attached_role(iam_instance_profile)
     role = options[:iam_role]
 
     if new_resource.role && !iam_instance_profile.roles.map(&:name).include?(role)
+      detach_role(iam_instance_profile)
       converge_by "associating role #{role} with instance profile #{new_resource.name}" do
         # Despite having collection methods for roles, instance profile can only have single role associated
-        detach_role(iam_instance_profile)
         iam_instance_profile.add_role({
           role_name: role
         })
@@ -33,23 +36,23 @@ def update_attached_role(iam_instance_profile)
   end
 
   def create_aws_object
-    iam = new_resource.driver.iam_resource
-
     converge_by "create IAM instance profile #{new_resource.name}" do
-      iam.create_instance_profile({
+      new_resource.driver.iam_resource.create_instance_profile({
         path: new_resource.path || "/",
         instance_profile_name: new_resource.name
       })
     end
   end
 
   def update_aws_object(iam_instance_profile)
-    update_attached_role(iam_instance_profile)
+    # Nothing to update on our object because the role relationship is managed
+    # through the action
+    iam_instance_profile
   end
 
   def destroy_aws_object(iam_instance_profile)
+    detach_role(iam_instance_profile)
     converge_by "delete #{iam_instance_profile.name}" do
-      detach_role(iam_instance_profile)
       iam_instance_profile.delete
     end
   end

lib/chef/provider/aws_iam_role.rb

@@ -1,40 +1,98 @@
 require 'chef/provisioning/aws_driver/aws_provider'
+require 'chef/json_compat'
 
 class Chef::Provider::AwsIamRole < Chef::Provisioning::AWSDriver::AWSProvider
   provides :aws_iam_role
 
+  def iam_client
+    new_resource.driver.iam_client
+  end
+
+  def iam_resource
+    new_resource.driver.iam_resource
+  end
+
+  def action_create
+    role = super
+
+    if !new_resource.inline_policies.nil?
+      update_inline_policy(role)
+    end
+  end
+
   protected
 
   def create_aws_object
-    iam = new_resource.driver.iam_resource
-
-    converge_by "create IAM role #{new_resource.name}" do
-      iam.create_role({
+    converge_by "create IAM Role #{new_resource.name}" do
+      iam_resource.create_role({
         path: new_resource.path,
         role_name: new_resource.name,
         assume_role_policy_document: new_resource.assume_role_policy_document
       })
     end
+    iam_resource.role(new_resource.name)
   end
 
-  def update_aws_object(iam_role)
+  def update_aws_object(role)
+    if new_resource.path && new_resource.path != role.path
+      raise "Path of IAM Role #{new_resource.name} is #{role.path}, but desired path is #{new_resource.path}.  IAM Role paths cannot be updated!"
+    end
+    if new_resource.assume_role_policy_document && policy_update_required?(role.assume_role_policy_document, new_resource.assume_role_policy_document)
+      converge_by "update IAM Role #{role.name} assume_role_policy_document" do
+        iam_client.update_assume_role_policy({
+          role_name: new_resource.name,
+          policy_document: new_resource.assume_role_policy_document
+        })
+      end
+    end
   end
 
-  def destroy_aws_object(iam_role)
-    converge_by "delete IAM role #{iam_role.name}" do
-      iam_role.instance_profiles.each do |profile|
-        profile.remove_role(role_name: iam_role.name)
+  def destroy_aws_object(role)
+    converge_by "delete IAM Role #{role.name}" do
+      role.instance_profiles.each do |profile|
+        profile.remove_role(role_name: role.name)
       end
-      iam_role.policies.each do |policy|
-        Cheffish.inline_resource(self, action) do
-          aws_iam_role_policy policy.name do
-            role iam_role.name
-            action :destroy
-          end
+      role.policies.each do |policy|
+        converge_by "delete IAM Role inline policy #{policy.name}" do
+          policy.delete
         end
       end
-      iam_role.delete
+      role.delete
     end
   end
 
+  private
+
+  def update_inline_policy(role)
+    desired_inline_policies = Hash[new_resource.inline_policies.map {|k, v| [k.to_s, v]}]
+    current_inline_policies = Hash[role.policies.map {|p| [p.name, p.policy_document]}]
+
+    policies_to_put = desired_inline_policies.reject {|k,v| current_inline_policies[k] && !policy_update_required?(current_inline_policies[k], v)}
+    policies_to_delete = current_inline_policies.keys - desired_inline_policies.keys
+
+    policies_to_put.each do |policy_name, policy|
+      converge_by "Adding or updating inline Role policy #{policy_name}" do
+        iam_client.put_role_policy({
+          role_name: role.name,
+          policy_name: policy_name,
+          policy_document: policy
+        })
+      end
+    end
+
+    policies_to_delete.each do |policy_name|
+      converge_by "Deleting inline Role policy #{policy_name}" do
+        iam_client.delete_role_policy({
+          role_name: role.name,
+          policy_name: policy_name
+        })
+      end
+    end
+  end
+
+  def policy_update_required?(current_policy, desired_policy)
+    # We parse the JSON into a hash to get rid of whitespace and ordering issues
+    Chef::JSONCompat.parse(URI.decode(current_policy)) != Chef::JSONCompat.parse(desired_policy)
+  end
+
 end

lib/chef/provisioning/aws_driver.rb

@@ -10,7 +10,6 @@
 require "chef/resource/aws_ebs_volume"
 require "chef/resource/aws_eip_address"
 require "chef/resource/aws_iam_role"
-require "chef/resource/aws_iam_role_policy"
 require "chef/resource/aws_iam_instance_profile"
 require "chef/resource/aws_image"
 require "chef/resource/aws_instance"

lib/chef/resource/aws_iam_role.rb

@@ -25,7 +25,26 @@ class Chef::Resource::AwsIamRole < Chef::Provisioning::AWSDriver::AWSResource
   #
   # The policy that grants an entity permission to assume the role.
   #
-  attribute :assume_role_policy_document, kind_of: String, required: true
+  attribute :assume_role_policy_document, kind_of: String
+
+  #
+  # Inline policies which _only_ apply to this role, unlike managed_policies
+  # which can be shared between users, groups and roles.  Maps to the
+  # [RolePolicy](http://docs.aws.amazon.com/sdkforruby/api/Aws/IAM/RolePolicy.html)
+  # SDK object.
+  #
+  # Hash keys are the inline policy name and the value is the policy document.
+  #
+  attribute :inline_policies, kind_of: Hash, callbacks: {
+    "inline_policies must be a hash maping policy names to policy documents" => proc do |policies|
+      policies.all? {|policy_name, policy| (policy_name.is_a?(String) || policy_name.is_a?(Symbol)) && policy.is_a?(String)}
+    end
+  }
+
+  #
+  # TODO: add when we get a policy resource
+  #
+  # attribute :managed_policies, kind_of: [Array, String, ::Aws::Iam::Policy, AwsIamPolicy], coerce: proc { |value| [value].flatten }
 
   def aws_object
     driver.iam_resource.role(name).load

spec/aws_support.rb

@@ -200,20 +200,20 @@ def self.included(context)
     Chef::Provisioning::AWSDriver::Resources.constants.each do |resource_class|
       resource_class = Chef::Provisioning::AWSDriver::Resources.const_get(resource_class)
       resource_name = resource_class.resource_name
-      define_method("update_an_#{resource_name}") do |name, expected_updates={}|
-        AWSSupport::Matchers::UpdateAnAWSObject.new(self, resource_class, name, expected_updates)
+      define_method("update_an_#{resource_name}") do |name, expected_updates={}, &block|
+        AWSSupport::Matchers::UpdateAnAWSObject.new(self, resource_class, name, expected_updates, block)
       end
-      define_method("create_an_#{resource_name}") do |name, expected_values={}|
-        AWSSupport::Matchers::CreateAnAWSObject.new(self, resource_class, name, expected_values)
+      define_method("create_an_#{resource_name}") do |name, expected_values={}, &block|
+        AWSSupport::Matchers::CreateAnAWSObject.new(self, resource_class, name, expected_values, block)
       end
       define_method("have_#{resource_name}_tags") do |name, expected_tags={}|
         AWSSupport::Matchers::HaveAWSObjectTags.new(self, resource_class, name, expected_tags)
       end
       define_method("destroy_an_#{resource_name}") do |name, expected_values={}|
         AWSSupport::Matchers::DestroyAnAWSObject.new(self, resource_class, name)
       end
-      define_method("match_an_#{resource_name}") do |name, expected_values={}|
-        AWSSupport::Matchers::MatchAnAWSObject.new(self, resource_class, name, expected_values)
+      define_method("match_an_#{resource_name}") do |name, expected_values={}, &block|
+        AWSSupport::Matchers::MatchAnAWSObject.new(self, resource_class, name, expected_values, block)
       end
     end
 

spec/aws_support/matchers/create_an_aws_object.rb

@@ -8,17 +8,21 @@ class CreateAnAWSObject
       include RSpec::Matchers::Composable
       include AWSSupport::DeepMatcher
 
-      def initialize(example, resource_class, name, expected_values)
+      # @param custom_matcher [Block] A block with 1 argument that will be provided the aws_obect
+      def initialize(example, resource_class, name, expected_values, custom_matcher)
         @example = example
         @resource_class = resource_class
         @name = name
         @expected_values = expected_values
+        @custom_matcher = custom_matcher
       end
 
       attr_reader :example
       attr_reader :resource_class
       attr_reader :name
       attr_reader :expected_values
+      attr_reader :custom_matcher
+
       def resource_name
         @resource_class.resource_name
       end
@@ -46,6 +50,8 @@ def match_failure_messages(recipe)
         resource.managed_entry_store Chef::Provisioning.chef_managed_entry_store
         aws_object = resource.aws_object
 
+        example.instance_exec aws_object, &custom_matcher if custom_matcher
+
         # Check existence
         if aws_object.nil?
           differences << "#{resource_name}[#{name}] was not created!"

spec/aws_support/matchers/match_an_aws_object.rb

@@ -13,17 +13,21 @@ class MatchAnAWSObject
       include RSpec::Matchers::Composable
       include AWSSupport::DeepMatcher
 
-      def initialize(example, resource_class, name, expected_values)
+      # @param custom_matcher [Block] A block with 1 argument that will be provided the aws_obect
+      def initialize(example, resource_class, name, expected_values, custom_matcher)
         @example = example
         @resource_class = resource_class
         @name = name
         @expected_values = expected_values
+        @custom_matcher = custom_matcher
       end
 
       attr_reader :example
       attr_reader :resource_class
       attr_reader :name
       attr_reader :expected_values
+      attr_reader :custom_matcher
+
       def resource_name
         @resource_class.resource_name
       end
@@ -44,6 +48,8 @@ def match_failure_messages(recipe)
         resource.managed_entry_store Chef::Provisioning.chef_managed_entry_store
         aws_object = resource.aws_object
 
+        example.instance_exec aws_object, &custom_matcher if custom_matcher
+
         # Check existence
         if aws_object.nil?
           differences << "#{resource_name}[#{name}] AWS object did not exist!"