Adds the ability to send to a different account id's queue.

[matthoey-okta]

I'm in no way, shape, or form a coder. I did my best to add a test but
failed to get the tests to run. Please feel free to fix any mistakes I
might have made. Looking forward to any feedback/help to get this
through.

Thanks for contributing to Logstash! If you haven't already signed our CLA, here's a handy link: https://www.elastic.co/contributor-agreement/

[robbavey]

Thank you for the contribution - just a couple of minor suggestions, but nothing major

[robbavey]

Maybe worth adding a note that the queue owner must grant permissions to access this queue.

[matthoey-okta]

Yes, this was a good call because I discovered both accounts need to grant permissions for this to work and that's not immediately obvious if you aren't familiar with cross account IAM permissions.

[robbavey]

It might be nice if the queue_owner_aws_account_id property only shows up if it has been explicitly set - otherwise this shows up in the logs as queue_owner_aws_account_id => nil

[NaomiAmethyst]

I suppose that would require either moving the logging inside the if statement:

if @queue_owner_aws_account_id
  @logger.debug('Connecting to cross-account SQS queue', :queue => @queue, :region => region, :queue_owner_aws_account_id => @queue_owner_aws_account_id)
  @queue_url = @sqs.get_queue_url(:queue_name => @queue, :queue_owner_aws_account_id => @queue_owner_aws_account_id)[:queue_url]
  @logger.info('Connected to cross-account SQS queue successfully', :queue => @queue, :region => region, :queue_owner_aws_account_id => @queue_owner_aws_account_id)
else
  @logger.debug('Connecting to SQS queue', :queue => @queue, :region => region)
  @queue_url = @sqs.get_queue_url(:queue_name => @queue)[:queue_url]
  @logger.info('Connected to SQS queue successfully', :queue => @queue, :region => region)
end

which seems a bit duplicative.

Or perhaps some form of hash manipulation:

queue_merger = @queue_owner_aws_account_id ? {:queue_owner_aws_account_id => @queue_owner_aws_account_id} : {}
@logger.debug('Connecting to SQS queue', {:queue => @queue, :region => region}.merge(queue_merger))
@queue_url = @sqs.get_queue_url({:queue_name => @queue}.merge(queue_merger))[:queue_url]
@logger.info('Connected to SQS queue successfully', {:queue => @queue, :region => region}.merge(queue_merger))

which is convoluted and messy.

Maybe a hybrid approach?

logger_params = { :queue => @queue, :region => region }
logger_params[:queue_owner_aws_account_id] = @queue_owner_aws_account_id if @queue_owner_aws_account_id
@logger.debug('Connecting to SQS queue', logger_params)
@queue_url = if @queue_owner_aws_account_id
  @sqs.get_queue_url(:queue_name => @queue, :queue_owner_aws_account_id => @queue_owner_aws_account_id)[:queue_url]
else
  @sqs.get_queue_url(:queue_name => @queue)[:queue_url]
end
@logger.info('Connected to SQS queue successfully', logger_params)

Any which way you slice it, it seems unnecessarily messy in the code for the value added in the logs.

[chrissivanich-okta]

Since :region isn't passed to sqs.get_queue_url couldn't @Cobi 's hybrid approach be used, but with passing the prepared hash into all called methods?

params = { queue: @queue }
params[:queue_owner_aws_account_id] = @queue_owner_aws_account_id if @queue_owner_aws_account_id
@logger.debug('Connecting to SQS queue', params)
@queue_url = @sqs.get_queue_url(params)
@logger.info('Connected to SQS queue successfully', params)

[matthoey-okta]

I think this closely resembles Chris' suggestion, but I felt it was a good balance of simplicity and fulfilling requirements. Note I did change the logging message to match the exact AWS attribute "queue_name" over the previously used "queue".

[robbavey]

@matthoey-okta Looks good to me! I will merge these changes shortly - thanks for the contribution

[elasticsearch-bot]

Rob Bavey merged this into the following branches!

Branch	Commits
master	f5db4c4, 984690b, 45b71ae, fc6e648