impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Laszlo Gaal (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-6067: Enable s3 access via IAM roles for EC2 VMs
Date Wed, 08 Nov 2017 21:40:40 GMT
Laszlo Gaal has posted comments on this change. ( )

Change subject: IMPALA-6067: Enable s3 access via IAM roles for EC2 VMs

Patch Set 2:

File bin/
PS2, Line 294:   if (set +x; [[ -z ${AWS_ACCESS_KEY_ID-} && -z ${AWS_SECRET_ACCESS_KEY-}
]]); then
> (I think it's fine if this is put in a separate script and called from buil
I'm not sure that putting the checks into a separate script would change anything. The part
in lines 252-264 have to remain here because they touch environment variables that are used
further in this script.
PS2, Line 294:   if (set +x; [[ -z ${AWS_ACCESS_KEY_ID-} && -z ${AWS_SECRET_ACCESS_KEY-}
]]); then
> Does really have to talk to the internet? It just tends to
In most of the use cases the network access is avoided, it happens only if the build is set
up to run the tests on S3.  -- and talking to S3 means lots of network access anyway.
Even the current version of the script performs a network access in the S3 case: line 321
uses AWS CLI to check the existence of the specified bucket.

Checking the credential URL is gated by the following environment variables:
- TARGET_FILESYSTEM has to be set to "s3"
- S3_BUCKET has to be non-empty
- AWS_SECRET_ACCESS_KEY and AWS_ACCESS_KEY_ID both have to be empty or missing
before the script attempts to look for credentials by checking the URL.

Even if a network check is performed, it will not go out to the internet. The specific network
address belongs to the "link-local address" category (see,
which is valid only in the immediate network neighborhood.
Within EC2 the request should be server quickly; outside of EC2 I don't expect the target
URL to exist at all. Based on this I could set short timeouts, so that the call fails quickly
even in rare failure cases.
PS2, Line 307:     if ! curl "${CURL_ARGS[@]}" ; then
> curl is not a development dependency in bin/ I'd sugges
Good point; I'll check if wget can be set up the same way.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I14cd9d4453a91baad3c379aa7e4944993fca95ae
Gerrit-Change-Number: 8294
Gerrit-PatchSet: 2
Gerrit-Owner: Laszlo Gaal <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: David Knupp <>
Gerrit-Reviewer: Jim Apple <>
Gerrit-Reviewer: Joe McDonnell <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Laszlo Gaal <>
Gerrit-Reviewer: Michael Brown <>
Gerrit-Reviewer: Philip Zeyliger <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Comment-Date: Wed, 08 Nov 2017 21:40:40 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message