search

concourse / registry-image-resource

a resource for images in a Docker registry
Apache License 2.0
89 stars 107 forks source link

Add support for: EC2 metadata AWS credentials and IAM roles chain #287

Closed fredericfran-gds closed 3 years ago

fredericfran-gds commented 3 years ago

This PR adds supports for:

  1. retrieving AWS credentials via EC2 metadata This is particularly useful if Concourse worker nodes are running on AWS EC2 instances with an IAM role attached already. This dispenses the need for potentially long life credentials to be passed to this resource. We include a new parameter: aws_ec2_credentials which will enable this feature.

  2. IAM roles chain In some scenarios, a shared Concourse is run in a separate AWS account (Account C) and need to assume a role in a different AWS account (Account A) in order to do the concourse tasks. The ECR is located in another AWS account: Account E. Only Account A has access to Account E. Therefore for registry-image to access ECR using the Account C credentials (available in EC2 metadata), it needs to assume a role in Account A which allows assuming a role in Account E have access to the ECR. We include a new parameter: aws_role_arns which takes a comma-delimited list of IAM roles to be assumed in the order specified. This overrides the existing aws_role_arn that we kept for backward compatibility.

aoldershaw commented 3 years ago

Hi @fredericfran-gds, thanks for submitting this. Unfortunately, I don't think we're going to accept it as is. Similar PRs have been closed for the same reason (#96, #254, concourse/s3-resource#115). For context as to why, take a look at concourse/concourse#3023. I'll try to summarize, though:

So, for a source to fully describe a resource that requires credentials to access, the source must contain those credentials. That said, we recognize that needing to create and maintain AWS credentials is a security hole that the EC2 metadata server helps solve. For this, we're currently working on a solution in Concourse itself to help deal with this, that's nicely described in https://github.com/concourse/concourse/issues/3023#issuecomment-603920117. This solution should hopefully fix both problems - you can still use the metadata server to fetch credentials, and the source still contains valid credentials.

We're still working toward this solution, though - so if you need this feature, and don't care about the downsides of sharing version history, I'd recommend building the registry-image-resource yourself using your fork and using that in your pipelines, at least for the time being.

Hope you understand where we're coming from!

fredericfran-gds commented 3 years ago

Hi @fredericfran-gds, thanks for submitting this. Unfortunately, I don't think we're going to accept it as is. Similar PRs have been closed for the same reason (#96, #254, concourse/s3-resource#115). For context as to why, take a look at concourse/concourse#3023. I'll try to summarize, though:

  • Concourse relies on the source as being the "source of truth" for the resource. That is, if two resources have the same source and the same type, then they must be the same resource
  • Concourse uses this assumption for several optimizations (identical resources in different pipelines are considered to be the same, and they share a version history)
  • Relying on worker state (like the fact that a worker can reach the EC2 metadata server) breaks this assumption, and can lead to information leakage (due to the shared version history)

So, for a source to fully describe a resource that requires credentials to access, the source must contain those credentials. That said, we recognize that needing to create and maintain AWS credentials is a security hole that the EC2 metadata server helps solve. For this, we're currently working on a solution in Concourse itself to help deal with this, that's nicely described in concourse/concourse#3023 (comment). This solution should hopefully fix both problems - you can still use the metadata server to fetch credentials, and the source still contains valid credentials.

We're still working toward this solution, though - so if you need this feature, and don't care about the downsides of sharing version history, I'd recommend building the registry-image-resource yourself using your fork and using that in your pipelines, at least for the time being.

Hope you understand where we're coming from!

Hi,

thank you for your comprehensive review. I understand the community position.

I would like to know what is your opinion on the second feature added by this PR.

Many thanks again.

Frederic.

aoldershaw commented 3 years ago

Oh my apologies, I didn't read it closely enough! That seems like a very reasonable extension of the existing aws_role_arn

fredericfran-gds commented 3 years ago

@aoldershaw I have decided to open a new PR here: https://github.com/concourse/registry-image-resource/pull/288 which includes all the changes that you recommended. Sorry if this is not an ideal workflow, I am learning about how to deal with forked & upstream repos etc. I'm sorry for any inconvenience.

aoldershaw commented 3 years ago

No worries! The whole fork workflow is pretty awkward. Thanks for the contribution!