You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by phrocker <gi...@git.apache.org> on 2018/06/26 16:26:58 UTC
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
GitHub user phrocker opened a pull request:
https://github.com/apache/nifi-minifi-cpp/pull/365
MINIFICPP-543: Implement C2 based update policy enforcement
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with MINIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFICPP-543
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi-minifi-cpp/pull/365.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 #365
----
commit c2223243058ecfdb78874600cce1723e0a99a470
Author: Marc Parisi <ph...@...>
Date: 2018-06-25T20:04:00Z
MINIFICPP-543: Implement C2 based update policy enforcement
----
---
[GitHub] nifi-minifi-cpp issue #365: MINIFICPP-543: Implement C2 based update policy ...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/365
reviewing
---
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/365#discussion_r199220210
--- Diff: libminifi/src/controllers/UpdatePolicyControllerService.cpp ---
@@ -0,0 +1,106 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "controllers/UpdatePolicyControllerService.h"
+#include <cstdio>
+#include <utility>
+#include <limits>
+#include <string>
+#include <vector>
+#include <sys/ioctl.h>
+#include <ifaddrs.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <set>
+#include "utils/StringUtils.h"
+#if ( defined(__APPLE__) || defined(__MACH__) || defined(BSD))
+#include <net/if_dl.h>
+#include <net/if_types.h>
+#endif
+#include "core/state/UpdatePolicy.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace controllers {
+
+core::Property UpdatePolicyControllerService::EnableAllProperties("Enable All Properties", "Enables all properties", "false");
+core::Property UpdatePolicyControllerService::AllowedProperties("Allowed Properties", "Properties for which we will allow updates");
+core::Property UpdatePolicyControllerService::DisallowedProperties("Disallowed Properties", "Properties for which we will not allow updates");
+core::Property UpdatePolicyControllerService::PersistUpdates("Persist Updates", "Property that dictates whether updates should persist a restart");
+
+void UpdatePolicyControllerService::initialize() {
+ std::set<core::Property> supportedProperties;
+ supportedProperties.insert(EnableAllProperties);
+ supportedProperties.insert(AllowedProperties);
+ supportedProperties.insert(DisallowedProperties);
+ supportedProperties.insert(PersistUpdates);
+ setSupportedProperties(supportedProperties);
+}
+
+void UpdatePolicyControllerService::yield() {
+}
+
+bool UpdatePolicyControllerService::isRunning() {
+ return getState() == core::controller::ControllerServiceState::ENABLED;
+}
+
+bool UpdatePolicyControllerService::isWorkAvailable() {
+ return false;
+}
+
+void UpdatePolicyControllerService::onEnable() {
+ std::string enableStr, persistStr;
+
+ bool enable_all = false;
+ if (getProperty(EnableAllProperties.getName(), enableStr)) {
+ enable_all = utils::StringUtils::StringToBool(enableStr, enable_all);
+ }
+
+ if (getProperty(PersistUpdates.getName(), persistStr)) {
+ persist_updates_ = utils::StringUtils::StringToBool(persistStr, persist_updates_);
+ }
+
+ auto builder = state::UpdatePolicyBuilder::newBuilder(enable_all);
+
+ core::Property all_prop("Allowed Properties", "Properties for which we will allow updates");
+ core::Property dall_prop("Disallowed Properties", "Properties for which we will allow updates");
--- End diff --
COPY PASTA! Thanks!
---
[GitHub] nifi-minifi-cpp issue #365: MINIFICPP-543: Implement C2 based update policy ...
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/365
@apiri I made some updates to the wiki entry.
---
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/365#discussion_r199212457
--- Diff: libminifi/src/controllers/UpdatePolicyControllerService.cpp ---
@@ -0,0 +1,106 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "controllers/UpdatePolicyControllerService.h"
+#include <cstdio>
+#include <utility>
+#include <limits>
+#include <string>
+#include <vector>
+#include <sys/ioctl.h>
+#include <ifaddrs.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <set>
+#include "utils/StringUtils.h"
+#if ( defined(__APPLE__) || defined(__MACH__) || defined(BSD))
+#include <net/if_dl.h>
+#include <net/if_types.h>
+#endif
+#include "core/state/UpdatePolicy.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace controllers {
+
+core::Property UpdatePolicyControllerService::EnableAllProperties("Enable All Properties", "Enables all properties", "false");
+core::Property UpdatePolicyControllerService::AllowedProperties("Allowed Properties", "Properties for which we will allow updates");
+core::Property UpdatePolicyControllerService::DisallowedProperties("Disallowed Properties", "Properties for which we will not allow updates");
+core::Property UpdatePolicyControllerService::PersistUpdates("Persist Updates", "Property that dictates whether updates should persist a restart");
+
+void UpdatePolicyControllerService::initialize() {
+ std::set<core::Property> supportedProperties;
+ supportedProperties.insert(EnableAllProperties);
+ supportedProperties.insert(AllowedProperties);
+ supportedProperties.insert(DisallowedProperties);
+ supportedProperties.insert(PersistUpdates);
+ setSupportedProperties(supportedProperties);
+}
+
+void UpdatePolicyControllerService::yield() {
+}
+
+bool UpdatePolicyControllerService::isRunning() {
+ return getState() == core::controller::ControllerServiceState::ENABLED;
+}
+
+bool UpdatePolicyControllerService::isWorkAvailable() {
+ return false;
+}
+
+void UpdatePolicyControllerService::onEnable() {
+ std::string enableStr, persistStr;
+
+ bool enable_all = false;
+ if (getProperty(EnableAllProperties.getName(), enableStr)) {
+ enable_all = utils::StringUtils::StringToBool(enableStr, enable_all);
+ }
+
+ if (getProperty(PersistUpdates.getName(), persistStr)) {
--- End diff --
I saw this is being handled via MINIFICPP-544. It seems like this should be used in conjunction with the temp/permanent considerations. Scanned through the C2 wiki page but am a little hazy about how this interacts with temp/permanent. I assume the policy service would get precedence but am uncertain of conditions such as I request allowing permanent but the instance is not configured to persist config changes.
---
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi-minifi-cpp/pull/365
---
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/365#discussion_r199214051
--- Diff: libminifi/src/controllers/UpdatePolicyControllerService.cpp ---
@@ -0,0 +1,106 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "controllers/UpdatePolicyControllerService.h"
+#include <cstdio>
+#include <utility>
+#include <limits>
+#include <string>
+#include <vector>
+#include <sys/ioctl.h>
+#include <ifaddrs.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <set>
+#include "utils/StringUtils.h"
+#if ( defined(__APPLE__) || defined(__MACH__) || defined(BSD))
+#include <net/if_dl.h>
+#include <net/if_types.h>
+#endif
+#include "core/state/UpdatePolicy.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace controllers {
+
+core::Property UpdatePolicyControllerService::EnableAllProperties("Enable All Properties", "Enables all properties", "false");
+core::Property UpdatePolicyControllerService::AllowedProperties("Allowed Properties", "Properties for which we will allow updates");
+core::Property UpdatePolicyControllerService::DisallowedProperties("Disallowed Properties", "Properties for which we will not allow updates");
+core::Property UpdatePolicyControllerService::PersistUpdates("Persist Updates", "Property that dictates whether updates should persist a restart");
+
+void UpdatePolicyControllerService::initialize() {
+ std::set<core::Property> supportedProperties;
+ supportedProperties.insert(EnableAllProperties);
+ supportedProperties.insert(AllowedProperties);
+ supportedProperties.insert(DisallowedProperties);
+ supportedProperties.insert(PersistUpdates);
+ setSupportedProperties(supportedProperties);
+}
+
+void UpdatePolicyControllerService::yield() {
+}
+
+bool UpdatePolicyControllerService::isRunning() {
+ return getState() == core::controller::ControllerServiceState::ENABLED;
+}
+
+bool UpdatePolicyControllerService::isWorkAvailable() {
+ return false;
+}
+
+void UpdatePolicyControllerService::onEnable() {
+ std::string enableStr, persistStr;
+
+ bool enable_all = false;
+ if (getProperty(EnableAllProperties.getName(), enableStr)) {
+ enable_all = utils::StringUtils::StringToBool(enableStr, enable_all);
+ }
+
+ if (getProperty(PersistUpdates.getName(), persistStr)) {
+ persist_updates_ = utils::StringUtils::StringToBool(persistStr, persist_updates_);
+ }
+
+ auto builder = state::UpdatePolicyBuilder::newBuilder(enable_all);
+
+ core::Property all_prop("Allowed Properties", "Properties for which we will allow updates");
+ core::Property dall_prop("Disallowed Properties", "Properties for which we will allow updates");
--- End diff --
allow -> disallow
---
[GitHub] nifi-minifi-cpp issue #365: MINIFICPP-543: Implement C2 based update policy ...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/365
@phrocker thank you for the updates, definitely clears things up. will get this merged in
---
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/365#discussion_r199220324
--- Diff: libminifi/test/unit/UpdatePolicyTests.cpp ---
@@ -0,0 +1,78 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <uuid/uuid.h>
+#include <vector>
+#include <memory>
+#include <utility>
+#include <string>
+#include "../TestBase.h"
+#include "io/ClientSocket.h"
+#include "core/Processor.h"
+#include "../../controller/Controller.h"
+#include "core/controller/ControllerService.h"
+#include "c2/ControllerSocketProtocol.h"
+#include "controllers/UpdatePolicyControllerService.h"
+
+TEST_CASE("TestEmptyPolicy", "[test1]") {
+ auto controller = std::make_shared<minifi::controllers::UpdatePolicyControllerService>("TestService");
+ std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ controller->initialize();
+ controller->onEnable();
+ REQUIRE(false == controller->canUpdate("anyproperty"));
+}
+
+TEST_CASE("TestEnableAll", "[test1]") {
+ auto controller = std::make_shared<minifi::controllers::UpdatePolicyControllerService>("TestService");
+ std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ controller->initialize();
+ controller->setProperty(minifi::controllers::UpdatePolicyControllerService::EnableAllProperties, "true");
+ controller->onEnable();
+ REQUIRE(true == controller->canUpdate("anyproperty"));
+}
+
+TEST_CASE("TestEnableAllFalls", "[test1]") {
--- End diff --
Oh wow. Thanks. This is what I get for using a 4k monitor.
---
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/365#discussion_r199218485
--- Diff: libminifi/src/controllers/UpdatePolicyControllerService.cpp ---
@@ -0,0 +1,106 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "controllers/UpdatePolicyControllerService.h"
+#include <cstdio>
+#include <utility>
+#include <limits>
+#include <string>
+#include <vector>
+#include <sys/ioctl.h>
+#include <ifaddrs.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <set>
+#include "utils/StringUtils.h"
+#if ( defined(__APPLE__) || defined(__MACH__) || defined(BSD))
+#include <net/if_dl.h>
+#include <net/if_types.h>
+#endif
+#include "core/state/UpdatePolicy.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace controllers {
+
+core::Property UpdatePolicyControllerService::EnableAllProperties("Enable All Properties", "Enables all properties", "false");
+core::Property UpdatePolicyControllerService::AllowedProperties("Allowed Properties", "Properties for which we will allow updates");
+core::Property UpdatePolicyControllerService::DisallowedProperties("Disallowed Properties", "Properties for which we will not allow updates");
+core::Property UpdatePolicyControllerService::PersistUpdates("Persist Updates", "Property that dictates whether updates should persist a restart");
--- End diff --
This is the intransitive verb.
---
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/365#discussion_r199213790
--- Diff: libminifi/test/unit/UpdatePolicyTests.cpp ---
@@ -0,0 +1,78 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <uuid/uuid.h>
+#include <vector>
+#include <memory>
+#include <utility>
+#include <string>
+#include "../TestBase.h"
+#include "io/ClientSocket.h"
+#include "core/Processor.h"
+#include "../../controller/Controller.h"
+#include "core/controller/ControllerService.h"
+#include "c2/ControllerSocketProtocol.h"
+#include "controllers/UpdatePolicyControllerService.h"
+
+TEST_CASE("TestEmptyPolicy", "[test1]") {
+ auto controller = std::make_shared<minifi::controllers::UpdatePolicyControllerService>("TestService");
+ std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ controller->initialize();
+ controller->onEnable();
+ REQUIRE(false == controller->canUpdate("anyproperty"));
+}
+
+TEST_CASE("TestEnableAll", "[test1]") {
+ auto controller = std::make_shared<minifi::controllers::UpdatePolicyControllerService>("TestService");
+ std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
+ controller->initialize();
+ controller->setProperty(minifi::controllers::UpdatePolicyControllerService::EnableAllProperties, "true");
+ controller->onEnable();
+ REQUIRE(true == controller->canUpdate("anyproperty"));
+}
+
+TEST_CASE("TestEnableAllFalls", "[test1]") {
--- End diff --
Falls -> Fails
---
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/365#discussion_r199214256
--- Diff: libminifi/src/controllers/UpdatePolicyControllerService.cpp ---
@@ -0,0 +1,106 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "controllers/UpdatePolicyControllerService.h"
+#include <cstdio>
+#include <utility>
+#include <limits>
+#include <string>
+#include <vector>
+#include <sys/ioctl.h>
+#include <ifaddrs.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <set>
+#include "utils/StringUtils.h"
+#if ( defined(__APPLE__) || defined(__MACH__) || defined(BSD))
+#include <net/if_dl.h>
+#include <net/if_types.h>
+#endif
+#include "core/state/UpdatePolicy.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace controllers {
+
+core::Property UpdatePolicyControllerService::EnableAllProperties("Enable All Properties", "Enables all properties", "false");
--- End diff --
Not sure I am tracking the delineation between "enable" and "allow". It seems that enable all might be better suited as AllowAll?
---
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/365#discussion_r199227402
--- Diff: libminifi/src/controllers/UpdatePolicyControllerService.cpp ---
@@ -0,0 +1,106 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "controllers/UpdatePolicyControllerService.h"
+#include <cstdio>
+#include <utility>
+#include <limits>
+#include <string>
+#include <vector>
+#include <sys/ioctl.h>
+#include <ifaddrs.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <set>
+#include "utils/StringUtils.h"
+#if ( defined(__APPLE__) || defined(__MACH__) || defined(BSD))
+#include <net/if_dl.h>
+#include <net/if_types.h>
+#endif
+#include "core/state/UpdatePolicy.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace controllers {
+
+core::Property UpdatePolicyControllerService::EnableAllProperties("Enable All Properties", "Enables all properties", "false");
--- End diff --
I typed a long response that Allow All doesn't fit what I intended with disallow, below; however, it was lost. I think the gist of that response was I will think about a better name to convey its meaning.
---
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/365#discussion_r199220156
--- Diff: libminifi/src/controllers/UpdatePolicyControllerService.cpp ---
@@ -0,0 +1,106 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "controllers/UpdatePolicyControllerService.h"
+#include <cstdio>
+#include <utility>
+#include <limits>
+#include <string>
+#include <vector>
+#include <sys/ioctl.h>
+#include <ifaddrs.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <set>
+#include "utils/StringUtils.h"
+#if ( defined(__APPLE__) || defined(__MACH__) || defined(BSD))
+#include <net/if_dl.h>
+#include <net/if_types.h>
+#endif
+#include "core/state/UpdatePolicy.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace controllers {
+
+core::Property UpdatePolicyControllerService::EnableAllProperties("Enable All Properties", "Enables all properties", "false");
+core::Property UpdatePolicyControllerService::AllowedProperties("Allowed Properties", "Properties for which we will allow updates");
+core::Property UpdatePolicyControllerService::DisallowedProperties("Disallowed Properties", "Properties for which we will not allow updates");
+core::Property UpdatePolicyControllerService::PersistUpdates("Persist Updates", "Property that dictates whether updates should persist a restart");
+
+void UpdatePolicyControllerService::initialize() {
+ std::set<core::Property> supportedProperties;
+ supportedProperties.insert(EnableAllProperties);
+ supportedProperties.insert(AllowedProperties);
+ supportedProperties.insert(DisallowedProperties);
+ supportedProperties.insert(PersistUpdates);
+ setSupportedProperties(supportedProperties);
+}
+
+void UpdatePolicyControllerService::yield() {
+}
+
+bool UpdatePolicyControllerService::isRunning() {
+ return getState() == core::controller::ControllerServiceState::ENABLED;
+}
+
+bool UpdatePolicyControllerService::isWorkAvailable() {
+ return false;
+}
+
+void UpdatePolicyControllerService::onEnable() {
+ std::string enableStr, persistStr;
+
+ bool enable_all = false;
+ if (getProperty(EnableAllProperties.getName(), enableStr)) {
+ enable_all = utils::StringUtils::StringToBool(enableStr, enable_all);
+ }
+
+ if (getProperty(PersistUpdates.getName(), persistStr)) {
--- End diff --
"I assume the policy service would get precedence but am uncertain of conditions such as I request allowing permanent but the instance is not configured to persist config changes."
I'm not sure I follow...but perhaps I should explain this a little better before adding a comment for the code to ensure it makes sense. Updates can be controlled via the flow via the controller service. There may be cases where you do not want configuration updates to be persisted. An easy case would be one in which there is no volatile storage. That's a bit trite as it over exemplifies the argument; however, a less whimsical example may be one in which you only want updates to persist through transferring the minifi.properties file. This ONLY controls updates performed through mechanisms that use the controller service. Does that make sense? If so I'd like to document that on the wiki and in the code as your question points out that it's a point of confusion.
---
[GitHub] nifi-minifi-cpp pull request #365: MINIFICPP-543: Implement C2 based update ...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/365#discussion_r199210517
--- Diff: libminifi/src/controllers/UpdatePolicyControllerService.cpp ---
@@ -0,0 +1,106 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "controllers/UpdatePolicyControllerService.h"
+#include <cstdio>
+#include <utility>
+#include <limits>
+#include <string>
+#include <vector>
+#include <sys/ioctl.h>
+#include <ifaddrs.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <set>
+#include "utils/StringUtils.h"
+#if ( defined(__APPLE__) || defined(__MACH__) || defined(BSD))
+#include <net/if_dl.h>
+#include <net/if_types.h>
+#endif
+#include "core/state/UpdatePolicy.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace controllers {
+
+core::Property UpdatePolicyControllerService::EnableAllProperties("Enable All Properties", "Enables all properties", "false");
+core::Property UpdatePolicyControllerService::AllowedProperties("Allowed Properties", "Properties for which we will allow updates");
+core::Property UpdatePolicyControllerService::DisallowedProperties("Disallowed Properties", "Properties for which we will not allow updates");
+core::Property UpdatePolicyControllerService::PersistUpdates("Persist Updates", "Property that dictates whether updates should persist a restart");
--- End diff --
a -> after
---