roadrunner-server / roadrunner

🤯 High-performance PHP application server, process manager written in Go and powered with plugins
https://docs.roadrunner.dev
MIT License
7.9k stars 411 forks source link

[🧹 CHORE]: Make SQS plugin check for IAM credentials for ECS #2029

Open nickdnk opened 18 hours ago

nickdnk commented 18 hours ago

No duplicates 🥲.

What should be improved or cleaned up?

It seems the SQS plugin only checks for EC2, not for ECS Fargate, when it is considering dynamic IAM credentials. This means that when running in containers, you would have to provide an explicit access key instead of relying on the IAM role for the task.

In order to use ECS credentials, the endpoint is 169.254.170.2 (and not 169.254.169.254) followed by the contents of the ENV variable AWS_CONTAINER_CREDENTIALS_RELATIVE_URI or just AWS_CONTAINER_CREDENTIALS_FULL_URI, if available. I'm unsure why both these variables exist, but it may be some BC compatibility.

The current implementation checks only the EC2 endpoint (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instancedata-data-retrieval.html) at https://github.com/roadrunner-server/sqs/blob/master/sqsjobs/driver.go#L32

You can see how the PHP SDK makes this call when used inside AWS ECS containers: https://github.com/aws/aws-sdk-php/blob/master/src/Credentials/EcsCredentialProvider.php#L192

You probably don't even need to hit the endpoint to check. If these variables are not available, it is not running in ECS:

public static function shouldUseEcs() {
  //Check for relative uri. if not, then full uri.
  //fall back to server for each as getenv is not thread-safe.
  return !empty(getenv(EcsCredentialProvider::ENV_URI))
      || !empty($_SERVER[EcsCredentialProvider::ENV_URI])
      || !empty(getenv(EcsCredentialProvider::ENV_FULL_URI))
      || !empty($_SERVER[EcsCredentialProvider::ENV_FULL_URI]);
}

I put this under Chore as it's somewhere between a Feature Request (IAM credentials already supported) and a bug (doesn't work if in an ECS container).

nickdnk commented 13 hours ago

Update. This is not actually a problem. The credentials in the check are not being used, they only determine the behavior of the sqs configuration parameters. I have made a PR in the sqs repo detailing this. See https://github.com/roadrunner-server/sqs/pull/570

nickdnk commented 8 hours ago

Second update: It actually is a problem, but for different reasons than originally stated, because of this code in the current version of the SQS plugin:

https://github.com/roadrunner-server/sqs/blob/e32b815a0edf4d20069e6234dd78ea86ee291c2f/sqsjobs/driver.go#L505-L521

As it doesn't detect ECS Fargate as an "AWS environment", it overrides the working credentials from the AWS SDK (from LoadDefaultConfig) with static ones (NewStaticCredentialsProvider), which are not provided to the ECS containers. My PR linked above also resolves this issues by simply getting completely rid of the logic that tries to determine if we're running in an AWS environment and putting that responsibility on the configuration instead.