Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201711082140.vA8Lee3b019476@ip-10-146-233-104.ec2.internal>
Date: Wed, 8 Nov 2017 21:40:40 +0000
From: "Laszlo Gaal (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, reviews@impala.incubator.apache.org
CC: Alex Behm <alex.behm@cloudera.com>, Sailesh Mukil <sailesh@cloudera.com>,
        Jim Apple <jbapple-impala@apache.org>, Lars Volker <lv@cloudera.com>,
        Michael Brown <mikeb@cloudera.com>,
        Philip Zeyliger <philip@cloudera.com>,
        David Knupp <dknupp@cloudera.com>,
        Joe McDonnell <joemcdonnell@cloudera.com>,
        Tim Armstrong <tarmstrong@cloudera.com>
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-6067=3A_Enable_s3_access_via_IAM_roles_for_EC2_VMs=0A?=
In-Reply-To: <gerrit.1508257187000.I14cd9d4453a91baad3c379aa7e4944993fca95ae@gerrit.cloudera.org>
References: <gerrit.1508257187000.I14cd9d4453a91baad3c379aa7e4944993fca95ae@gerrit.cloudera.org>
Reply-To: laszlo.gaal@cloudera.com, sailesh@cloudera.com, dknupp@cloudera.com,
        philip@cloudera.com, jbapple-impala@apache.org, alex.behm@cloudera.com,
        reviews@impala.incubator.apache.org, mikeb@cloudera.com,
        impala-cr@cloudera.com, lv@cloudera.com, marcelk@gmail.com,
        tarmstrong@cloudera.com, joemcdonnell@cloudera.com
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.14.2
Content-Type: multipart/alternative; boundary="7BuI1/8dlfA="; charset=UTF-8
archived-at: Wed, 08 Nov 2017 21:40:48 -0000

--7BuI1/8dlfA=
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Laszlo Gaal has posted comments on this change=2E ( http://gerrit=2Eclouder=
a=2Eorg:8080/8294 )

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


Patch Set 2:

(3 comments)

http://gerrit=2Ecloudera=2Eorg:8080/#/c/8=
294/2/bin/impala-config=2Esh
File bin/impala-config=2Esh:

http://gerrit=2E=
cloudera=2Eorg:8080/#/c/8294/2/bin/impala-config=2Esh@294
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=2E The part in lines 252-264 have to remain here because t=
hey touch environment variables that are used further in this script=2E


h=
ttp://gerrit=2Ecloudera=2Eorg:8080/#/c/8294/2/bin/impala-config=2Esh@294
PS=
2, Line 294:   if (set +x; [[ -z ${AWS_ACCESS_KEY_ID-} && -z ${AWS_SECRET_A=
CCESS_KEY-} ]]); then
> Does impala-config=2Esh really have to talk to the =
internet? It just tends to
In most of the use cases the network access is a=
voided, it happens only if the build is set up to run the tests on S3=2E  -=
- and talking to S3 means lots of network access anyway=2E
Even the current=
 version of the script performs a network access in the S3 case: line 321 u=
ses AWS CLI to check the existence of the specified bucket=2E

Checking the=
 credential URL is gated by the following environment variables:
- TARGET_F=
ILESYSTEM has to be set to "s3"
- S3_BUCKET has to be non-empty
- AWS_SECRE=
T_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=2E

Even if=
 a network check is performed, it will not go out to the internet=2E The sp=
ecific network address belongs to the "link-local address" category (see ht=
tps://en=2Ewikipedia=2Eorg/wiki/Link-local_address), which is valid only in=
 the immediate network neighborhood=2E
Within EC2 the request should be ser=
ver quickly; outside of EC2 I don't expect the target URL to exist at all=
=2E Based on this I could set short timeouts, so that the call fails quickl=
y even in rare failure cases=2E


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8=
294/2/bin/impala-config=2Esh@307
PS2, Line 307:     if ! curl "${CURL_ARGS[=
@]}" ; then
> curl is not a development dependency in bin/bootstrap_system=
=2Esh=2E I'd sugges
Good point; I'll check if wget can be set up the same w=
ay=2E


-- 
To view, visit http://gerrit=2Ecloudera=2Eorg:8080/8294
To uns=
ubscribe, visit http://gerrit=2Ecloudera=2Eorg:8080/settings

Gerrit-Projec=
t: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Chan=
ge-Id: I14cd9d4453a91baad3c379aa7e4944993fca95ae
Gerrit-Change-Number: 8294=

Gerrit-PatchSet: 2
Gerrit-Owner: Laszlo Gaal <laszlo=2Egaal@cloudera=2Ecom=
>
Gerrit-Reviewer: Alex Behm <alex=2Ebehm@cloudera=2Ecom>
Gerrit-Reviewer: =
David Knupp <dknupp@cloudera=2Ecom>
Gerrit-Reviewer: Jim Apple <jbapple-imp=
ala@apache=2Eorg>
Gerrit-Reviewer: Joe McDonnell <joemcdonnell@cloudera=2Ec=
om>
Gerrit-Reviewer: Lars Volker <lv@cloudera=2Ecom>
Gerrit-Reviewer: Laszl=
o Gaal <laszlo=2Egaal@cloudera=2Ecom>
Gerrit-Reviewer: Michael Brown <mikeb=
@cloudera=2Ecom>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera=2Ecom>
G=
errit-Reviewer: Sailesh Mukil <sailesh@cloudera=2Ecom>
Gerrit-Reviewer: Tim=
 Armstrong <tarmstrong@cloudera=2Ecom>
Gerrit-Comment-Date: Wed, 08 Nov 201=
7 21:40:40 +0000
Gerrit-HasComments: Yes

--7BuI1/8dlfA=--