Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vsis3: incomplete support for container credentials #8858

Closed
moradology opened this issue Nov 28, 2023 · 6 comments
Closed

vsis3: incomplete support for container credentials #8858

moradology opened this issue Nov 28, 2023 · 6 comments
Assignees

Comments

@moradology
Copy link

moradology commented Nov 28, 2023

Expected behavior and actual behavior.

I expected vsis3 to transparently acquire credentials from the environment as it does on EC2 and as generally works on container services provided by AWS (e.g. ECS). On EMR-Serverless, however, this fails to happen.

Enabling logging and CURL logging, it becomes obvious that GDAL is failing to connect to the expected metadata/credentials endpoint:

[30;1m [45;1m[1 of 1000] [0m [36;1mDEBUG(1) CPLE_None(0) "No error." AWS: AWS_ROLE_ARN configuration option not defined [0m
HTTP: Fetch(http://169.254.169.254/latest/api/token)
HTTP: libcurl/8.4.0 OpenSSL/3.1.4 zlib/1.2.13 zstd/1.5.5 libssh2/1.11.0 nghttp2/1.57.0
CURL_INFO_TEXT: Couldn't find host 169.254.169.254 in the .netrc file; using defaults
CURL_INFO_TEXT:   Trying 169.254.169.254:80...
CURL_INFO_TEXT: Immediate connect fail for [169.254.169.254](http://169.254.169.254/): Invalid argument
CURL_INFO_TEXT: Failed to connect to 169.254.169.254 port 80 after 0 ms: Couldn't connect to server
CURL_INFO_TEXT: Closing connection
HTTP: Fetch(http://169.254.169.254/latest/meta-data/iam/security-credentials/)
CURL_INFO_TEXT: Couldn't find host 169.254.169.254 in the .netrc file; using defaults
CURL_INFO_TEXT:   Trying 169.254.169.254:80...
CURL_INFO_TEXT: Immediate connect fail for [169.254.169.254](http://169.254.169.254/): Invalid argument
CURL_INFO_TEXT: Failed to connect to 169.254.169.254 port 80 after 0 ms: Couldn't connect to server
CURL_INFO_TEXT: Closing connection

Digging in further, it appears to be the case that this section of code is incomplete as AWS_CONTAINER_CREDENTIALS_FULL_URI is not referenced:
https://github.com/OSGeo/gdal/blob/master/port/cpl_aws.cpp#L835C8-L849

Unfortunately, the documentation doesn't really describe when/where FULL vs RELATIVE environment variables are to be expected. Unhelpfully, the upshot of this documentation is "FULL might be used if RELATIVE isn't".
image

Reviewing the environment in the cluster I deployed, it becomes clear that this is exactly the issue:
image

Steps to reproduce the problem.

Try to use GDAL's vsis3 support on EMR-Serverless. It will 403.

Operating system

All operating systems.

GDAL version and provenance

Every single version of GDAL up to and including master.

How to resolve

The resolution here is relatively simple, I think. cpl_aws.cpp simply needs to have a bit more logic that enables it to use AWS_CONTAINER_CREDENTIALS_FULL_URI should that be provided in the container environment.

@rouault rouault self-assigned this Nov 28, 2023
@rouault
Copy link
Member

rouault commented Nov 28, 2023

@moradology Thanks for the detailed analysis. Could you try the patch in #8859 ? I've only unit tested it as I don't have easy access to a EMR-Serverless environment

@moradology
Copy link
Author

moradology commented Nov 28, 2023

And here I was getting excited about lodging my first gdal PR 🤣

Yeah, I'll test this out!

@moradology
Copy link
Author

moradology commented Nov 28, 2023

OK, so the good news in terms of me not writing it is that I apparently have no taste at all when it comes to C++

Here's the diff I had:

diff --git a/port/cpl_aws.cpp b/port/cpl_aws.cpp
index 9aaa8178ee..eb1650a421 100644
--- a/port/cpl_aws.cpp
+++ b/port/cpl_aws.cpp
@@ -840,8 +840,16 @@ bool VSIS3HandleHelper::GetConfigurationFromEC2(
     // coverity[tainted_data]
     const CPLString osECSRelativeURI(VSIGetPathSpecificOption(
         osPathForOption.c_str(), "AWS_CONTAINER_CREDENTIALS_RELATIVE_URI", ""));
+    const CPLString osFullCredentialsURI(VSIGetPathSpecificOption(
+        osPathForOption.c_str(), "AWS_CONTAINER_CREDENTIALS_FULL_URI", ""));
     CPLString osToken;
-    if (osEC2RootURL == osEC2DefaultURL && !osECSRelativeURI.empty())
+    if (!osFullCredentialsURI.empty())
+    {
+        // See
+        // https://docs.aws.amazon.com/sdkref/latest/guide/feature-container-credentials.html
+        osURLRefreshCredentials = osFullCredentialsURI;
+    }
+    else if (osEC2RootURL == osEC2DefaultURL && !osECSRelativeURI.empty())
     {
         // See
         // https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-iam-roles.html

@rouault
Copy link
Member

rouault commented Nov 28, 2023

And here I was getting excited about lodging my first gdal PR 🤣

oops, I thought that you filed an issue because you didn't feel like doing a PR... I should have asked.

OK, so the good news in terms of me not writing it is that I apparently have no taste at all when it comes to C++

well, your proposal looks equally good. The only difference in mine is that I don't fetch AWS_CONTAINER_CREDENTIALS_RELATIVE_URI if we already get a non empty AWS_CONTAINER_CREDENTIALS_FULL_URI, but that's a micro optimization.

@moradology
Copy link
Author

moradology commented Nov 28, 2023

You're too kind. Sorry this is taking a bit - gotta build your branch on a specific base image, put it on ecr, write a little program for the jvm, etc

Let me also say this: libs on amazon linux available via yum are ancient!

@moradology
Copy link
Author

moradology commented Nov 29, 2023

Tested with cross account permissions and role assumption. We're in business:

package com.example.geotrellis

import scala.sys.process._

object GdalInfo {
  def main(args: Array[String]): Unit = {
    if (args.length != 1) {
      println("Usage: GdalInfoApp <URL>")
    } else {
      val url = args(0)
      val gdalInfoCmd = s"gdalinfo $url"

      try {
        val output = gdalInfoCmd.!!
        println(output)
      } catch {
        case e: Exception => println(s"Error running gdalinfo: ${e.getMessage}")
      }
    }
  }
}
image image

rouault added a commit that referenced this issue Nov 29, 2023
/vsis3/: takes into account AWS_CONTAINER_CREDENTIALS_FULL_URI environment variable (fixes #8858)
rouault added a commit that referenced this issue Nov 29, 2023
rouault added a commit that referenced this issue Nov 30, 2023
[Backport release/3.8] /vsis3/: takes into account AWS_CONTAINER_CREDENTIALS_FULL_URI environment variable (fixes #8858)
ralphraul pushed a commit to 1SpatialGroupLtd/gdal that referenced this issue Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants