Terminated instances included in aws_ec2_instance(s) results

[mwiczynski-cartera]

When an ec2 instance is terminated it can still appear in results/response for describing ec2 instances.

Describe the problem

When an ec2 instance is terminated it can still appear in results/response for describing ec2 instances. Eventually AWS removes it. A terminated instance passes the it { should exist }. This is not correct. it can also lead to problems when you have more than one instance with same name. One is terminated and other is non-terminated state(running, stopped, etc). The aws_ec2_instance(name: 'foo') resource will fail because more than once instance matching name is returned. The aws_ec2_instances resource does not allow filter by instance state. The resources should filter out terminated instances by default or give ability to do so. Not sure if anyone would ever want them to be included given how transient the terminated state is.

Possible Solution

I suggest filtering out terminated by default. I suppose could also add instance state as column in FilterTable?

Change fetch_data https://github.com/inspec/inspec-aws/blob/a23887fa38bd5ae277a4c93d5e6c1b8ba3db4a2b/libraries/aws_ec2_instances.rb#L30 to filter out terminated.

 def fetch_data
    instance_rows = []
    filters = [
      {
        name: 'instance-state-name',
        values: [
          'pending',
          'running',
          'shutting-down',
          'stopping',
          'stopped'
        ]
      }]
    options = {filters: filters}

    loop do
      catch_aws_errors do
        @api_response = @aws.compute_client.describe_instances(options)
      end
      return [] if !@api_response || @api_response.empty?
      @api_response.reservations.each do |res|
        res.instances.each do |instance|
          instance_tags = map_tags(instance.tags)
          instance_rows += [{
            instance_id: instance.instance_id,
            vpc_id: instance.vpc_id,
            subnet_id: instance.subnet_id,
            instance_type: instance.instance_type,
            tags: instance_tags,
            name: instance_tags["Name"],
            iam_profile: instance.iam_instance_profile&.arn&.split("/")&.last,
          }]
        end
      end
      break unless @api_response.next_token
      options = { next_token: @api_response.next_token }
    end
    @table = instance_rows
  end

This might not be ideal but I like idea of filtering out terminated instance here https://github.com/inspec/inspec-aws/blob/a23887fa38bd5ae277a4c93d5e6c1b8ba3db4a2b/libraries/aws_ec2_instance.rb#L19

  def initialize(opts = {})
    opts = { instance_id: opts } if opts.is_a?(String)
    super(opts)
    validate_parameters(require_any_of: %i(instance_id name))

    state_filter =  {
      name: 'instance-state-name',
      values: [
        'pending',
        'running',
        'shutting-down',
        'stopping',
        'stopped'
      ]
    }

    if opts[:instance_id] && !opts[:instance_id].empty? # Use instance_id, if provided
      if !opts[:instance_id].is_a?(String) || opts[:instance_id] !~ /(^i-[0-9a-f]{8})|(^i-[0-9a-f]{17})$/
        raise ArgumentError, "#{@__resource_name__}: `instance_id` must be a string in the format of 'i-' followed by 8 or 17 hexadecimal characters."
      end
      @display_name = opts[:instance_id]
      instance_arguments = { instance_ids: [opts[:instance_id]], filters: [state_filter] }
    elsif opts[:name] && !opts[:name].empty? # Otherwise use name, if provided
      @display_name = opts[:name]
      instance_arguments = { filters: [{ name: "tag:Name", values: [opts[:name]] }, state_filter] }
    else
      raise ArgumentError, "#{@__resource_name__}: either instance_id or name must be provided."
    end

[aaronlippold]

If you fork the repo and push a small PR with the update that would be the fastest way to see this improvement moved into the resource pack.

[clintoncwolfe]

I think this is a great suggestion! This would indeed make a great PR.