You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2017/10/30 18:15:13 UTC

Review Request 63423: Cleared the executor auth token after using it.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63423/
-----------------------------------------------------------

Review request for mesos and Greg Mann.


Bugs: MESOS-8140
    https://issues.apache.org/jira/browse/MESOS-8140


Repository: mesos


Description
-------

Since the executor may run in the smae container and under
the same credential as the user task, we should clear the
`MESOS_EXECUTOR_AUTHENTICATION_TOKEN` environment variable
once we sample it.


Diffs
-----

  src/executor/executor.cpp 7280951909b677e2a7b0afbcddba4a299dbe6f06 
  src/launcher/default_executor.cpp cdb3c3e919a94996bba323fa04bfdc927d537910 


Diff: https://reviews.apache.org/r/63423/diff/1/


Testing
-------

make check (Fedora 26)

Manual inspection of child process metadata.


Thanks,

James Peach


Re: Review Request 63423: Cleared the executor auth token after using it.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63423/#review195156
-----------------------------------------------------------



FAIL: mesos-java failed to build.

Reviews applied: `['63422', '63423']`

Failed command: `cmake.exe --build . --target mesos-java`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63423

Relevant logs:

- [mesos-java-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63423/logs/mesos-java-build-cmake-stdout.log):

```
"D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj" (default target) (10) ->
  LINK : fatal error LNK1104: cannot open file [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]
  LINK : fatal error LNK1104: cannot open file 'C:\Users\mesos\AppData\Local\Temp\lnk{0A79E57C-31B0-4470-BF07-F949EE91311C}.tmp' [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta\src\libevent-2.1.5-beta-build\CMakeFiles\CMakeTmp\cmTC_6d466.vcxproj] [D:\DCOS\mesos\3rdparty\libevent-2.1.5-beta.vcxproj]


"D:\DCOS\mesos\src\java\mesos-java.vcxproj" (default target) (1) ->
"D:\DCOS\mesos\src\java\mesos-jar.vcxproj" (default target) (13) ->
"D:\DCOS\mesos\src\mesos-protobufs.vcxproj" (default target) (14) ->
"D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj" (default target) (17) ->
  LINK : fatal error LNK1104: cannot open file [D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj]
  LINK : fatal error LNK1104: cannot open file 'C:\Users\mesos\AppData\Local\Temp\lnk{9D4C7090-BD5B-4AB8-8947-B1480913EE26}.tmp' [D:\DCOS\mesos\3rdparty\protobuf-3.5.0\src\protobuf-3.5.0-build\CMakeFiles\CMakeTmp\cmTC_2006e.vcxproj] [D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj]


"D:\DCOS\mesos\src\java\mesos-java.vcxproj" (default target) (1) ->
"D:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj" (default target) (19) ->
"D:\DCOS\mesos\3rdparty\zlib-1.2.8.vcxproj" (default target) (20) ->
  LINK : fatal error LNK1104: cannot open file [D:\DCOS\mesos\3rdparty\zlib-1.2.8.vcxproj]
  LINK : fatal error LNK1104: cannot open file 'C:\Users\mesos\AppData\Local\Temp\lnk{03CAD12A-0C00-440D-8AA8-3B975C88E25C}.tmp' [D:\DCOS\mesos\3rdparty\zlib-1.2.8\src\zlib-1.2.8-build\CMakeFiles\CMakeTmp\cmTC_67fa9.vcxproj] [D:\DCOS\mesos\3rdparty\zlib-1.2.8.vcxproj]


"D:\DCOS\mesos\src\java\mesos-java.vcxproj" (default target) (1) ->
"D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj" (default target) (21) ->
  LINK : fatal error LNK1104: cannot open file [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
  LINK : fatal error LNK1104: cannot open file 'C:\Users\mesos\AppData\Local\Temp\lnk{CA2FF784-98E4-4EE5-AE04-ECC075104346}.tmp' [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\CMakeFiles\CMakeTmp\cmTC_c7105.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]

    1 Warning(s)
    15 Error(s)

Time Elapsed 00:03:47.53
```

- [mesos-java-build-CMakeOutput.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63423/logs/mesos-java-build-CMakeOutput.log):

```
  Creating directory "cmTC_4e8bb.dir\Debug\".

  Creating directory "D:\DCOS\mesos\CMakeFiles\CMakeTmp\Debug\".

  Creating directory "cmTC_4e8bb.dir\Debug\cmTC_4e8bb.tlog\".

InitializeBuildStatus:

  Creating "cmTC_4e8bb.dir\Debug\cmTC_4e8bb.tlog\unsuccessfulbuild" because "AlwaysCreate" was specified.

ClCompile:

  C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.12.25827\bin\HostX64\x64\CL.exe /c /Zi /W3 /WX- /diagnostics:classic /Od /Ob0 /D WIN32 /D _WINDOWS /D COMPILER_SUPPORTS_CXX11 /D "CMAKE_INTDIR=\"Debug\"" /D _MBCS /Gm- /EHsc /RTC1 /MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR /Fo"cmTC_4e8bb.dir\Debug\" /Fd"cmTC_4e8bb.dir\Debug\vc141.pdb" /Gd /TP /errorReport:queue D:\DCOS\mesos\CMakeFiles\CMakeTmp\src.cxx

  Microsoft (R) C/C++ Optimizing Compiler Version 19.12.25830.2 for x64

  Copyright (C) Microsoft Corporation.  All rights reserved.

  

  cl /c /Zi /W3 /WX- /diagnostics:classic /Od /Ob0 /D WIN32 /D _WINDOWS /D COMPILER_SUPPORTS_CXX11 /D "CMAKE_INTDIR=\"Debug\"" /D _MBCS /Gm- /EHsc /RTC1 /MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR /Fo"cmTC_4e8bb.dir\Debug\" /Fd"cmTC_4e8bb.dir\Debug\vc141.pdb" /Gd /TP /errorReport:queue D:\DCOS\mesos\CMakeFiles\CMakeTmp\src.cxx

  src.cxx

  

Link:

  C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.12.25827\bin\HostX64\x64\link.exe /ERRORREPORT:QUEUE /OUT:"D:\DCOS\mesos\CMakeFiles\CMakeTmp\Debug\cmTC_4e8bb.exe" /INCREMENTAL /NOLOGO kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTUAC:"level='asInvoker' uiAccess='false'" /manifest:embed /DEBUG /PDB:"D:/DCOS/mesos/CMakeFiles/CMakeTmp/Debug/cmTC_4e8bb.pdb" /SUBSYSTEM:CONSOLE /TLBID:1 /DYNAMICBASE /NXCOMPAT /IMPLIB:"D:/DCOS/mesos/CMakeFiles/CMakeTmp/Debug/cmTC_4e8bb.lib" /MACHINE:X64  /machine:x64 cmTC_4e8bb.dir\Debug\src.obj

  cmTC_4e8bb.vcxproj -> D:\DCOS\mesos\CMakeFiles\CMakeTmp\Debug\cmTC_4e8bb.exe

FinalizeBuildStatus:

  Deleting file "cmTC_4e8bb.dir\Debug\cmTC_4e8bb.tlog\unsuccessfulbuild".

  Touching "cmTC_4e8bb.dir\Debug\cmTC_4e8bb.tlog\cmTC_4e8bb.lastbuildstate".

Done Building Project "D:\DCOS\mesos\CMakeFiles\CMakeTmp\cmTC_4e8bb.vcxproj" (default targets).



Build succeeded.

    0 Warning(s)

    0 Error(s)



Time Elapsed 00:00:01.18


Source file was:
int main() { return 0; }
```

- [mesos-java-build-CMakeError.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63423/logs/mesos-java-build-CMakeError.log):

```
PrepareForBuild:

  Creating directory "cmTC_1292d.dir\Debug\".

  Creating directory "D:\DCOS\mesos\CMakeFiles\CMakeTmp\Debug\".

  Creating directory "cmTC_1292d.dir\Debug\cmTC_1292d.tlog\".

InitializeBuildStatus:

  Creating "cmTC_1292d.dir\Debug\cmTC_1292d.tlog\unsuccessfulbuild" because "AlwaysCreate" was specified.

ClCompile:

  C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.12.25827\bin\HostX64\x64\CL.exe /c /Zi /W3 /WX- /diagnostics:classic /MP /Od /Ob0 /D WIN32 /D _WINDOWS /D UNICODE /D _UNICODE /D "CMAKE_INTDIR=\"Debug\"" /D _UNICODE /D UNICODE /Gm- /RTC1 /MTd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /Fo"cmTC_1292d.dir\Debug\" /Fd"cmTC_1292d.dir\Debug\vc141.pdb" /Gd /TC /errorReport:queue D:\DCOS\mesos\CMakeFiles\CMakeTmp\CheckIncludeFile.c

  Microsoft (R) C/C++ Optimizing Compiler Version 19.12.25830.2 for x64

  Copyright (C) Microsoft Corporation.  All rights reserved.

  

  cl /c /Zi /W3 /WX- /diagnostics:classic /MP /Od /Ob0 /D WIN32 /D _WINDOWS /D UNICODE /D _UNICODE /D "CMAKE_INTDIR=\"Debug\"" /D _UNICODE /D UNICODE /Gm- /RTC1 /MTd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /Fo"cmTC_1292d.dir\Debug\" /Fd"cmTC_1292d.dir\Debug\vc141.pdb" /Gd /TC /errorReport:queue D:\DCOS\mesos\CMakeFiles\CMakeTmp\CheckIncludeFile.c

  CheckIncludeFile.c

  

D:\DCOS\mesos\CMakeFiles\CMakeTmp\CheckIncludeFile.c(1): fatal error C1083: Cannot open include file: 'pthread.h': No such file or directory [D:\DCOS\mesos\CMakeFiles\CMakeTmp\cmTC_1292d.vcxproj]

Done Building Project "D:\DCOS\mesos\CMakeFiles\CMakeTmp\cmTC_1292d.vcxproj" (default targets) -- FAILED.



Build FAILED.



"D:\DCOS\mesos\CMakeFiles\CMakeTmp\cmTC_1292d.vcxproj" (default target) (1) ->

(ClCompile target) -> 

  D:\DCOS\mesos\CMakeFiles\CMakeTmp\CheckIncludeFile.c(1): fatal error C1083: Cannot open include file: 'pthread.h': No such file or directory [D:\DCOS\mesos\CMakeFiles\CMakeTmp\cmTC_1292d.vcxproj]



    0 Warning(s)

    1 Error(s)



Time Elapsed 00:00:00.60



```

- Mesos Reviewbot Windows


On Jan. 10, 2018, 4:37 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63423/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2018, 4:37 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8140
>     https://issues.apache.org/jira/browse/MESOS-8140
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the executor may run in the smae container and under
> the same credential as the user task, we should clear the
> `MESOS_EXECUTOR_AUTHENTICATION_TOKEN` environment variable
> once we sample it.
> 
> 
> Diffs
> -----
> 
>   src/executor/executor.cpp 1c972d92984a15d12466e3dd229b16fa60e21ebb 
> 
> 
> Diff: https://reviews.apache.org/r/63423/diff/3/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> Manual inspection of child process metadata.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 63423: Cleared the executor auth token after using it.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63423/#review196071
-----------------------------------------------------------


Ship it!




Ship It!

- Greg Mann


On Jan. 10, 2018, 4:37 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63423/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2018, 4:37 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8140
>     https://issues.apache.org/jira/browse/MESOS-8140
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the executor may run in the smae container and under
> the same credential as the user task, we should clear the
> `MESOS_EXECUTOR_AUTHENTICATION_TOKEN` environment variable
> once we sample it.
> 
> 
> Diffs
> -----
> 
>   src/executor/executor.cpp 1c972d92984a15d12466e3dd229b16fa60e21ebb 
> 
> 
> Diff: https://reviews.apache.org/r/63423/diff/3/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> Manual inspection of child process metadata.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 63423: Cleared the executor auth token after using it.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63423/
-----------------------------------------------------------

(Updated Jan. 10, 2018, 4:37 p.m.)


Review request for mesos and Greg Mann.


Bugs: MESOS-8140
    https://issues.apache.org/jira/browse/MESOS-8140


Repository: mesos


Description
-------

Since the executor may run in the smae container and under
the same credential as the user task, we should clear the
`MESOS_EXECUTOR_AUTHENTICATION_TOKEN` environment variable
once we sample it.


Diffs (updated)
-----

  src/executor/executor.cpp 1c972d92984a15d12466e3dd229b16fa60e21ebb 


Diff: https://reviews.apache.org/r/63423/diff/3/

Changes: https://reviews.apache.org/r/63423/diff/2-3/


Testing
-------

make check (Fedora 26)

Manual inspection of child process metadata.


Thanks,

James Peach


Re: Review Request 63423: Cleared the executor auth token after using it.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63423/
-----------------------------------------------------------

(Updated Jan. 3, 2018, 10:42 p.m.)


Review request for mesos and Greg Mann.


Bugs: MESOS-8140
    https://issues.apache.org/jira/browse/MESOS-8140


Repository: mesos


Description
-------

Since the executor may run in the smae container and under
the same credential as the user task, we should clear the
`MESOS_EXECUTOR_AUTHENTICATION_TOKEN` environment variable
once we sample it.


Diffs (updated)
-----

  src/executor/executor.cpp 1c972d92984a15d12466e3dd229b16fa60e21ebb 


Diff: https://reviews.apache.org/r/63423/diff/2/

Changes: https://reviews.apache.org/r/63423/diff/1-2/


Testing
-------

make check (Fedora 26)

Manual inspection of child process metadata.


Thanks,

James Peach


Re: Review Request 63423: Cleared the executor auth token after using it.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63423/#review190076
-----------------------------------------------------------




src/launcher/default_executor.cpp
Lines 1604 (patched)
<https://reviews.apache.org/r/63423/#comment267332>

    Ah, I had forgotten that we store the authorization header here for use by the checker/health checker, and then the default executor instantiates the executor library, which grabs the token from the environment again. This isn't ideal, but we'll probably have to refactor this anyway when we start to use the authenticatee module here.
    
    So, for now we can just eliminate this line and let the executor library unset the env var when it's loaded. Otherwise, the default executor tests fail because the executor library can't authenticate.


- Greg Mann


On Oct. 30, 2017, 6:15 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63423/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2017, 6:15 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8140
>     https://issues.apache.org/jira/browse/MESOS-8140
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the executor may run in the smae container and under
> the same credential as the user task, we should clear the
> `MESOS_EXECUTOR_AUTHENTICATION_TOKEN` environment variable
> once we sample it.
> 
> 
> Diffs
> -----
> 
>   src/executor/executor.cpp 7280951909b677e2a7b0afbcddba4a299dbe6f06 
>   src/launcher/default_executor.cpp cdb3c3e919a94996bba323fa04bfdc927d537910 
> 
> 
> Diff: https://reviews.apache.org/r/63423/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> Manual inspection of child process metadata.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 63423: Cleared the executor auth token after using it.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63423/#review189715
-----------------------------------------------------------



PASS: Mesos patch 63423 was successfully built and tested.

Reviews applied: `['63422', '63423']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63423

- Mesos Reviewbot Windows


On Oct. 30, 2017, 11:15 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63423/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2017, 11:15 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8140
>     https://issues.apache.org/jira/browse/MESOS-8140
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the executor may run in the smae container and under
> the same credential as the user task, we should clear the
> `MESOS_EXECUTOR_AUTHENTICATION_TOKEN` environment variable
> once we sample it.
> 
> 
> Diffs
> -----
> 
>   src/executor/executor.cpp 7280951909b677e2a7b0afbcddba4a299dbe6f06 
>   src/launcher/default_executor.cpp cdb3c3e919a94996bba323fa04bfdc927d537910 
> 
> 
> Diff: https://reviews.apache.org/r/63423/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> Manual inspection of child process metadata.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 63423: Cleared the executor auth token after using it.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63423/#review189681
-----------------------------------------------------------



Patch looks great!

Reviews applied: [63422, 63423]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Oct. 30, 2017, 6:15 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63423/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2017, 6:15 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8140
>     https://issues.apache.org/jira/browse/MESOS-8140
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the executor may run in the smae container and under
> the same credential as the user task, we should clear the
> `MESOS_EXECUTOR_AUTHENTICATION_TOKEN` environment variable
> once we sample it.
> 
> 
> Diffs
> -----
> 
>   src/executor/executor.cpp 7280951909b677e2a7b0afbcddba4a299dbe6f06 
>   src/launcher/default_executor.cpp cdb3c3e919a94996bba323fa04bfdc927d537910 
> 
> 
> Diff: https://reviews.apache.org/r/63423/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26)
> 
> Manual inspection of child process metadata.
> 
> 
> Thanks,
> 
> James Peach
> 
>