You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Attila Magyar <am...@hortonworks.com> on 2017/03/31 15:57:00 UTC
Re: Review Request 58079: Cleanup temporary files needed for
downloading client configurations response
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58079/
-----------------------------------------------------------
(Updated March 31, 2017, 3:57 p.m.)
Review request for Ambari, Attila Doroszlai, Bal�zs Bence S�ri, Laszlo Puskas, Robert Levas, and Sebastian Toader.
Changes
-------
cleaning up command json files
Bugs: AMBARI-20596
https://issues.apache.org/jira/browse/AMBARI-20596
Repository: ambari
Description (updated)
-------
A tar.gz file is generated during client configuration download. This file is not cleand up and its file permission is world-readable, but should be owner-readable.
Diffs (updated)
-----
ambari-common/src/main/python/resource_management/libraries/script/script.py fad14fd
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java e98c062
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java 8f0c66f
ambari-server/src/test/python/stacks/2.0.6/HDFS/test_hdfs_client.py b2636ab
Diff: https://reviews.apache.org/r/58079/diff/2/
Changes: https://reviews.apache.org/r/58079/diff/1-2/
Testing
-------
modified existing unittest, tested client config download manually:
- created cluster with yarn
- downloaded client configuration
- checked file permission
[root@c6401 vagrant]# ls -al /var/lib/ambari-server/data/tmp/*.tar.gz
-rw------- 1 root root 6834 Mar 31 11:53 /var/lib/ambari-server/data/tmp/YARN_CLIENT-configs.tar.gz
existing tests: passed
Thanks,
Attila Magyar
Re: Review Request 58079: Cleanup temporary files needed for
downloading client configurations response
Posted by Sebastian Toader <st...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58079/#review170736
-----------------------------------------------------------
Ship it!
Ship It!
- Sebastian Toader
On March 31, 2017, 5:57 p.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58079/
> -----------------------------------------------------------
>
> (Updated March 31, 2017, 5:57 p.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Bal�zs Bence S�ri, Laszlo Puskas, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-20596
> https://issues.apache.org/jira/browse/AMBARI-20596
>
>
> Repository: ambari
>
>
> Description
> -------
>
> A tar.gz file is generated during client configuration download. This file is not cleand up and its file permission is world-readable, but should be owner-readable.
>
>
> Diffs
> -----
>
> ambari-common/src/main/python/resource_management/libraries/script/script.py fad14fd
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java e98c062
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java 8f0c66f
> ambari-server/src/test/python/stacks/2.0.6/HDFS/test_hdfs_client.py b2636ab
>
>
> Diff: https://reviews.apache.org/r/58079/diff/2/
>
>
> Testing
> -------
>
> modified existing unittest, tested client config download manually:
> - created cluster with yarn
> - downloaded client configuration
> - checked file permission
>
> [root@c6401 vagrant]# ls -al /var/lib/ambari-server/data/tmp/*.tar.gz
> -rw------- 1 root root 6834 Mar 31 11:53 /var/lib/ambari-server/data/tmp/YARN_CLIENT-configs.tar.gz
>
>
> existing tests: passed
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 58079: Cleanup temporary files needed for
downloading client configurations response
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58079/#review170759
-----------------------------------------------------------
Ship it!
I tested out the patch. Nice job.
- Robert Levas
On March 31, 2017, 11:57 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58079/
> -----------------------------------------------------------
>
> (Updated March 31, 2017, 11:57 a.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Bal�zs Bence S�ri, Laszlo Puskas, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-20596
> https://issues.apache.org/jira/browse/AMBARI-20596
>
>
> Repository: ambari
>
>
> Description
> -------
>
> A tar.gz file is generated during client configuration download. This file is not cleand up and its file permission is world-readable, but should be owner-readable.
>
>
> Diffs
> -----
>
> ambari-common/src/main/python/resource_management/libraries/script/script.py fad14fd
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java e98c062
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java 8f0c66f
> ambari-server/src/test/python/stacks/2.0.6/HDFS/test_hdfs_client.py b2636ab
>
>
> Diff: https://reviews.apache.org/r/58079/diff/2/
>
>
> Testing
> -------
>
> modified existing unittest, tested client config download manually:
> - created cluster with yarn
> - downloaded client configuration
> - checked file permission
>
> [root@c6401 vagrant]# ls -al /var/lib/ambari-server/data/tmp/*.tar.gz
> -rw------- 1 root root 6834 Mar 31 11:53 /var/lib/ambari-server/data/tmp/YARN_CLIENT-configs.tar.gz
>
>
> existing tests: passed
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 58079: Cleanup temporary files needed for
downloading client configurations response
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58079/#review170872
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Levas
On April 3, 2017, 6 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58079/
> -----------------------------------------------------------
>
> (Updated April 3, 2017, 6 a.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Bal�zs Bence S�ri, Laszlo Puskas, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-20596
> https://issues.apache.org/jira/browse/AMBARI-20596
>
>
> Repository: ambari
>
>
> Description
> -------
>
> A tar.gz file is generated during client configuration download. This file is not cleand up and its file permission is world-readable, but should be owner-readable.
>
>
> Diffs
> -----
>
> ambari-common/src/main/python/resource_management/libraries/script/script.py fad14fd
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java e98c062
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java 8f0c66f
> ambari-server/src/test/python/stacks/2.0.6/HDFS/test_hdfs_client.py b2636ab
>
>
> Diff: https://reviews.apache.org/r/58079/diff/3/
>
>
> Testing
> -------
>
> modified existing unittest, tested client config download manually:
> - created cluster with yarn
> - downloaded client configuration
> - checked file permission
>
> [root@c6401 vagrant]# ls -al /var/lib/ambari-server/data/tmp/*.tar.gz
> -rw------- 1 root root 6834 Mar 31 11:53 /var/lib/ambari-server/data/tmp/YARN_CLIENT-configs.tar.gz
>
>
> existing tests: passed
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 58079: Cleanup temporary files needed for
downloading client configurations response
Posted by Attila Magyar <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58079/
-----------------------------------------------------------
(Updated April 3, 2017, 10 a.m.)
Review request for Ambari, Attila Doroszlai, Bal�zs Bence S�ri, Laszlo Puskas, Robert Levas, and Sebastian Toader.
Changes
-------
Attila D. found issues with putting 0600 on the directory. Changed this to 0700.
Bugs: AMBARI-20596
https://issues.apache.org/jira/browse/AMBARI-20596
Repository: ambari
Description
-------
A tar.gz file is generated during client configuration download. This file is not cleand up and its file permission is world-readable, but should be owner-readable.
Diffs (updated)
-----
ambari-common/src/main/python/resource_management/libraries/script/script.py fad14fd
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java e98c062
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java 8f0c66f
ambari-server/src/test/python/stacks/2.0.6/HDFS/test_hdfs_client.py b2636ab
Diff: https://reviews.apache.org/r/58079/diff/3/
Changes: https://reviews.apache.org/r/58079/diff/2-3/
Testing
-------
modified existing unittest, tested client config download manually:
- created cluster with yarn
- downloaded client configuration
- checked file permission
[root@c6401 vagrant]# ls -al /var/lib/ambari-server/data/tmp/*.tar.gz
-rw------- 1 root root 6834 Mar 31 11:53 /var/lib/ambari-server/data/tmp/YARN_CLIENT-configs.tar.gz
existing tests: passed
Thanks,
Attila Magyar
Re: Review Request 58079: Cleanup temporary files needed for
downloading client configurations response
Posted by Attila Magyar <am...@hortonworks.com>.
> On March 31, 2017, 5:45 p.m., Robert Levas wrote:
> > ambari-common/src/main/python/resource_management/libraries/script/script.py
> > Lines 877 (patched)
> > <https://reviews.apache.org/r/58079/diff/2/?file=1682424#file1682424line877>
> >
> > It is not a good idea to chmod 600 the directory set as the Ambari server's temporary directory. This, by default is `/var/lib/ambari-server/data/tmp`, but may be changed using the `server.tmp.dir` property in `ambari-server.properties` file. So someone could set this a `/var/tmp`.
> >
> > Ideally the _correct_ permission was set on this directory when it was created.
The conf_tmp_dir is not the ambari temp directory, but a random subdirectory created under the ambari temp dir.
conf_tmp_dir = tempfile.mkdtemp(dir=self.get_tmp_dir()) # <- this creates a temp dir in ambar tmp
- Attila
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58079/#review170747
-----------------------------------------------------------
On March 31, 2017, 3:57 p.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58079/
> -----------------------------------------------------------
>
> (Updated March 31, 2017, 3:57 p.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Bal�zs Bence S�ri, Laszlo Puskas, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-20596
> https://issues.apache.org/jira/browse/AMBARI-20596
>
>
> Repository: ambari
>
>
> Description
> -------
>
> A tar.gz file is generated during client configuration download. This file is not cleand up and its file permission is world-readable, but should be owner-readable.
>
>
> Diffs
> -----
>
> ambari-common/src/main/python/resource_management/libraries/script/script.py fad14fd
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java e98c062
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java 8f0c66f
> ambari-server/src/test/python/stacks/2.0.6/HDFS/test_hdfs_client.py b2636ab
>
>
> Diff: https://reviews.apache.org/r/58079/diff/2/
>
>
> Testing
> -------
>
> modified existing unittest, tested client config download manually:
> - created cluster with yarn
> - downloaded client configuration
> - checked file permission
>
> [root@c6401 vagrant]# ls -al /var/lib/ambari-server/data/tmp/*.tar.gz
> -rw------- 1 root root 6834 Mar 31 11:53 /var/lib/ambari-server/data/tmp/YARN_CLIENT-configs.tar.gz
>
>
> existing tests: passed
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 58079: Cleanup temporary files needed for
downloading client configurations response
Posted by Robert Levas <rl...@hortonworks.com>.
> On March 31, 2017, 1:45 p.m., Robert Levas wrote:
> > ambari-common/src/main/python/resource_management/libraries/script/script.py
> > Lines 877 (patched)
> > <https://reviews.apache.org/r/58079/diff/2/?file=1682424#file1682424line877>
> >
> > It is not a good idea to chmod 600 the directory set as the Ambari server's temporary directory. This, by default is `/var/lib/ambari-server/data/tmp`, but may be changed using the `server.tmp.dir` property in `ambari-server.properties` file. So someone could set this a `/var/tmp`.
> >
> > Ideally the _correct_ permission was set on this directory when it was created.
>
> Attila Magyar wrote:
> The conf_tmp_dir is not the ambari temp directory, but a random subdirectory created under the ambari temp dir.
>
> conf_tmp_dir = tempfile.mkdtemp(dir=self.get_tmp_dir()) # <- this creates a temp dir in ambar tmp
Thanks for the clarification. I will drop this.
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58079/#review170747
-----------------------------------------------------------
On March 31, 2017, 11:57 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58079/
> -----------------------------------------------------------
>
> (Updated March 31, 2017, 11:57 a.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Bal�zs Bence S�ri, Laszlo Puskas, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-20596
> https://issues.apache.org/jira/browse/AMBARI-20596
>
>
> Repository: ambari
>
>
> Description
> -------
>
> A tar.gz file is generated during client configuration download. This file is not cleand up and its file permission is world-readable, but should be owner-readable.
>
>
> Diffs
> -----
>
> ambari-common/src/main/python/resource_management/libraries/script/script.py fad14fd
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java e98c062
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java 8f0c66f
> ambari-server/src/test/python/stacks/2.0.6/HDFS/test_hdfs_client.py b2636ab
>
>
> Diff: https://reviews.apache.org/r/58079/diff/2/
>
>
> Testing
> -------
>
> modified existing unittest, tested client config download manually:
> - created cluster with yarn
> - downloaded client configuration
> - checked file permission
>
> [root@c6401 vagrant]# ls -al /var/lib/ambari-server/data/tmp/*.tar.gz
> -rw------- 1 root root 6834 Mar 31 11:53 /var/lib/ambari-server/data/tmp/YARN_CLIENT-configs.tar.gz
>
>
> existing tests: passed
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 58079: Cleanup temporary files needed for
downloading client configurations response
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58079/#review170747
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
ambari-common/src/main/python/resource_management/libraries/script/script.py
Lines 877 (patched)
<https://reviews.apache.org/r/58079/#comment243634>
It is not a good idea to chmod 600 the directory set as the Ambari server's temporary directory. This, by default is `/var/lib/ambari-server/data/tmp`, but may be changed using the `server.tmp.dir` property in `ambari-server.properties` file. So someone could set this a `/var/tmp`.
Ideally the _correct_ permission was set on this directory when it was created.
- Robert Levas
On March 31, 2017, 11:57 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58079/
> -----------------------------------------------------------
>
> (Updated March 31, 2017, 11:57 a.m.)
>
>
> Review request for Ambari, Attila Doroszlai, Bal�zs Bence S�ri, Laszlo Puskas, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-20596
> https://issues.apache.org/jira/browse/AMBARI-20596
>
>
> Repository: ambari
>
>
> Description
> -------
>
> A tar.gz file is generated during client configuration download. This file is not cleand up and its file permission is world-readable, but should be owner-readable.
>
>
> Diffs
> -----
>
> ambari-common/src/main/python/resource_management/libraries/script/script.py fad14fd
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java e98c062
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java 8f0c66f
> ambari-server/src/test/python/stacks/2.0.6/HDFS/test_hdfs_client.py b2636ab
>
>
> Diff: https://reviews.apache.org/r/58079/diff/2/
>
>
> Testing
> -------
>
> modified existing unittest, tested client config download manually:
> - created cluster with yarn
> - downloaded client configuration
> - checked file permission
>
> [root@c6401 vagrant]# ls -al /var/lib/ambari-server/data/tmp/*.tar.gz
> -rw------- 1 root root 6834 Mar 31 11:53 /var/lib/ambari-server/data/tmp/YARN_CLIENT-configs.tar.gz
>
>
> existing tests: passed
>
>
> Thanks,
>
> Attila Magyar
>
>