You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by JPercivall <gi...@git.apache.org> on 2016/11/29 14:46:03 UTC
[GitHub] nifi-minifi pull request #59: MINIFI-125 Migrating the Bootstrap and Resourc...
GitHub user JPercivall opened a pull request:
https://github.com/apache/nifi-minifi/pull/59
MINIFI-125 Migrating the Bootstrap and Resources modules changes from NiFi 1.0 and 1.1
This does not include the changes for: variable registry and encrypted properties. Both are much bigger efforts that deserve their own ticket.
Also this does not include PR 1092 due to the issue found with it (NIFI-3112)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/JPercivall/nifi-minifi MINIFI-125
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi-minifi/pull/59.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #59
----
commit f69c128c98fcfd78cddde567a1582dc589a0abc3
Author: Joseph Percivall <jp...@apache.org>
Date: 2016-11-28T23:13:34Z
MINIFI-126 Migrating improvments made in NiFi PR 386
commit 6af750a18c8da6490d4bf8e335c480c69e399c17
Author: Joseph Percivall <jp...@apache.org>
Date: 2016-11-28T23:22:09Z
Migrating improvements made in NiFi PR 668
commit b46ce879874df49dc1dceeb268b97e33f396e778
Author: Joseph Percivall <jp...@apache.org>
Date: 2016-11-28T23:23:28Z
Migrating improvements made in NiFi PR 677
commit 9e0d7bc96cc5f589fbde9dad197dc24ed3004cde
Author: Joseph Percivall <jp...@apache.org>
Date: 2016-11-28T23:41:36Z
Migrating the style improvements made in NiFi PR 834. Not implementing security upgrade as it's too large and important to be in this commit. Separate effort should be done to incorporate the rest of the PR.
commit ee9bbc0c834f58802a86d1c54ea4c2826f21c63d
Author: Joseph Percivall <jp...@apache.org>
Date: 2016-11-28T23:46:29Z
Migrating improvements made in NiFi PR 1093 and 1159
commit 9c70a49e0a741f5e09388ba04d2895dd57f44bd9
Author: Joseph Percivall <jp...@apache.org>
Date: 2016-11-29T00:20:41Z
Fixing a couple NiFi -> MiNiFi class name issues
commit ad77732f022917df9009ff4c05fa182d9d4953ae
Author: Joseph Percivall <jp...@apache.org>
Date: 2016-11-29T00:22:39Z
Migrating improvements made in NiFi PR 383
commit fecba42e6cc0e42580d3e7132ded75055e15ec90
Author: Joseph Percivall <jp...@apache.org>
Date: 2016-11-29T00:26:12Z
Migrating improvements made in NiFi PR 414
commit ef3f1b76a783c41a70961da3b58aa47d1bd9f197
Author: Joseph Percivall <jp...@apache.org>
Date: 2016-11-29T00:32:24Z
Migrating improvements made in NiFi PR 561
commit 387566b3dbc077f91d229736ccc671042186910f
Author: Joseph Percivall <jp...@apache.org>
Date: 2016-11-29T00:42:19Z
Migrating improvements made in NiFi PR 614
commit aa7ad384f62a29d03ec69be393f8c2221e767ce1
Author: Joseph Percivall <jp...@apache.org>
Date: 2016-11-29T00:50:21Z
Migrating improvements made in NiFi PR 1137
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request #59: MINIFI-125 Migrating the Bootstrap and Resourc...
Posted by brosander <gi...@git.apache.org>.
Github user brosander commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/59#discussion_r90285106
--- Diff: minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/RunMiNiFi.java ---
@@ -367,11 +399,40 @@ private synchronized void saveProperties(final Properties nifiProps, final Logge
}
try (final FileOutputStream fos = new FileOutputStream(statusFile)) {
- nifiProps.store(fos, null);
+ minifiProps.store(fos, null);
fos.getFD().sync();
}
- logger.debug("Saved Properties {} to {}", new Object[]{nifiProps, statusFile});
+ logger.debug("Saved Properties {} to {}", new Object[]{minifiProps, statusFile});
+ }
+
+ private synchronized void writePidFile(final String pid, final Logger logger) throws IOException {
+ final File pidFile = getPidFile(logger);
+ if (pidFile.exists() && !pidFile.delete()) {
--- End diff --
Fair enough, I'm ok with it for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request #59: MINIFI-125 Migrating the Bootstrap and Resourc...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi-minifi/pull/59
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request #59: MINIFI-125 Migrating the Bootstrap and Resourc...
Posted by brosander <gi...@git.apache.org>.
Github user brosander commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/59#discussion_r90112258
--- Diff: minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/RunMiNiFi.java ---
@@ -357,8 +387,10 @@ private synchronized void saveProperties(final Properties nifiProps, final Logge
try {
final Set<PosixFilePermission> perms = new HashSet<>();
- perms.add(PosixFilePermission.OWNER_READ);
perms.add(PosixFilePermission.OWNER_WRITE);
+ perms.add(PosixFilePermission.OWNER_READ);
+ perms.add(PosixFilePermission.GROUP_READ);
+ perms.add(PosixFilePermission.OTHERS_READ);
--- End diff --
@JPercivall maybe not relevant in MiNiFi context but when I see "others" getting permissions I always wonder why it's necessary and if any sensitive data can leak.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request #59: MINIFI-125 Migrating the Bootstrap and Resourc...
Posted by brosander <gi...@git.apache.org>.
Github user brosander commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/59#discussion_r90240544
--- Diff: minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/RunMiNiFi.java ---
@@ -357,8 +387,10 @@ private synchronized void saveProperties(final Properties nifiProps, final Logge
try {
final Set<PosixFilePermission> perms = new HashSet<>();
- perms.add(PosixFilePermission.OWNER_READ);
perms.add(PosixFilePermission.OWNER_WRITE);
+ perms.add(PosixFilePermission.OWNER_READ);
+ perms.add(PosixFilePermission.GROUP_READ);
+ perms.add(PosixFilePermission.OTHERS_READ);
--- End diff --
On further investigation, it seems like this is just the pid file (I initially assumed it was the properties file)
I'm ok with others knowing the pid since OS permissions should still keep them from doing bad things. (Also, it's not uncommon to be able to list all processes)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request #59: MINIFI-125 Migrating the Bootstrap and Resourc...
Posted by brosander <gi...@git.apache.org>.
Github user brosander commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/59#discussion_r90290047
--- Diff: minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/RunMiNiFi.java ---
@@ -367,11 +399,40 @@ private synchronized void saveProperties(final Properties nifiProps, final Logge
}
try (final FileOutputStream fos = new FileOutputStream(statusFile)) {
- nifiProps.store(fos, null);
+ minifiProps.store(fos, null);
fos.getFD().sync();
}
- logger.debug("Saved Properties {} to {}", new Object[]{nifiProps, statusFile});
+ logger.debug("Saved Properties {} to {}", new Object[]{minifiProps, statusFile});
+ }
+
+ private synchronized void writePidFile(final String pid, final Logger logger) throws IOException {
+ final File pidFile = getPidFile(logger);
+ if (pidFile.exists() && !pidFile.delete()) {
--- End diff --
Makes sense
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi-minifi pull request #59: MINIFI-125 Migrating the Bootstrap and Resourc...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/59#discussion_r90129643
--- Diff: minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/RunMiNiFi.java ---
@@ -367,11 +399,40 @@ private synchronized void saveProperties(final Properties nifiProps, final Logge
}
try (final FileOutputStream fos = new FileOutputStream(statusFile)) {
- nifiProps.store(fos, null);
+ minifiProps.store(fos, null);
fos.getFD().sync();
}
- logger.debug("Saved Properties {} to {}", new Object[]{nifiProps, statusFile});
+ logger.debug("Saved Properties {} to {}", new Object[]{minifiProps, statusFile});
+ }
+
+ private synchronized void writePidFile(final String pid, final Logger logger) throws IOException {
+ final File pidFile = getPidFile(logger);
+ if (pidFile.exists() && !pidFile.delete()) {
--- End diff --
I really don't think there is anything we can do about it since each invocation of minifi.sh is a separate processes. The lock file has the same potential race condition[1].
[1] https://github.com/apache/nifi-minifi/blob/master/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/RunMiNiFi.java#L751
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---