[GitHub] nifi-registry issue #149: NIFIREG-215 Extension Bundle Improvements
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/149 Thanks, @bbende! +1, merged to master. ---
[GitHub] nifi-registry pull request #149: NIFIREG-215 Extension Bundle Improvements
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/149#discussion_r241217517 --- Diff: nifi-registry-core/nifi-registry-data-model/src/main/java/org/apache/nifi/registry/bucket/Bucket.java --- @@ -41,6 +41,8 @@ private String description; +private Boolean allowExtensionBundleRedeploy; --- End diff -- Ok, Iâm not familiar with Nexusâs configuration options, but Iâm in favor of keeping consistencies/parallels with that. Iâm good with leaving this as-is. ---
[GitHub] nifi-registry pull request #149: NIFIREG-215 Extension Bundle Improvements
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/149#discussion_r241172387
--- Diff:
nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/ExtensionResource.java
---
@@ -93,15 +94,25 @@ public ExtensionResource(final RegistryService
registryService,
responseContainer = "List"
)
@ApiResponses({ @ApiResponse(code = 401, message =
HttpStatusMessages.MESSAGE_401) })
-public Response getExtensionBundles() {
+public Response getExtensionBundles(
+@QueryParam("groupId")
+@ApiParam("Optional groupId to filter results. The value may
be an exact match, or a trailing wildcard, " +
+"such as 'com.%' to select all bundles where the
groupId starts with 'com.'.")
+final String groupId,
+@QueryParam("artifactId")
+@ApiParam("Optional artifactId to filter results. The value
may be an exact match, or a trailing wildcard, " +
+"such as 'nifi-%' to select all bundles where the
artifactId starts with 'nifi-'.")
+final String artifactId) {
--- End diff --
These parameters are nice to have, and work as expected for me. One thing I
noticed is that the documentation says _"... a trailing wildcard ..."_, which
indicates wildcards can only be used at the end of the value. However in
testing, I was able to use them anywhere in the value (at the beginning or
middle of the value) and it worked (with the expected results as well).
So maybe just update the documentation to remove the work "trailing"?
---
[GitHub] nifi-registry pull request #149: NIFIREG-215 Extension Bundle Improvements
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/149#discussion_r241166717 --- Diff: nifi-registry-core/nifi-registry-data-model/src/main/java/org/apache/nifi/registry/bucket/Bucket.java --- @@ -41,6 +41,8 @@ private String description; +private Boolean allowExtensionBundleRedeploy; --- End diff -- Instead of a boolean flag here. I'm wondering if this would be better as a Enum for something like "redeploy policy" with values such as "NEVER, SNAPSHOTS, ALWAYS" or something like that? ---
[GitHub] nifi-registry issue #149: NIFIREG-215 Extension Bundle Improvements
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/149 Reviewing... ---
[GitHub] nifi-registry issue #151: Bugfix overriding db props via environment variabl...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/151 Thanks for the contribution @jgondron. Will review this when I get a chance! ---
[GitHub] nifi-registry pull request #148: NIFIREG-211 Initial work for adding extenio...
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/148#discussion_r237568373 --- Diff: nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/extension/StandardExtensionService.java --- @@ -188,13 +188,14 @@ public ExtensionBundleVersion createExtensionBundleVersion(final String bucketId // create the bundle version in the metadata db final String userIdentity = NiFiUserUtils.getNiFiUserIdentity(); +final long bundleCreatedTime = extensionBundle.getCreated().getTime(); final ExtensionBundleVersionMetadata versionMetadata = new ExtensionBundleVersionMetadata(); versionMetadata.setId(UUID.randomUUID().toString()); versionMetadata.setExtensionBundleId(extensionBundle.getId()); versionMetadata.setBucketId(bucketIdentifier); versionMetadata.setVersion(version); -versionMetadata.setTimestamp(System.currentTimeMillis()); +versionMetadata.setTimestamp(bundleCreatedTime); --- End diff -- This works great when the bundle is being created with the first version uploaded. But I noticed when subsequent versions are uploaded they are also getting tagged with the same creation time. Given that we don't know what was done in getOrCreateExtensionBundle, we might need to refactor the code above, or... it might be better to just revert this change to always use currentTime for the bundle version. I'm good with either ---
[GitHub] nifi-registry pull request #148: NIFIREG-211 Initial work for adding extenio...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/148#discussion_r236810679
--- Diff:
nifi-registry-core/nifi-registry-data-model/src/main/java/org/apache/nifi/registry/extension/repo/ExtensionRepoVersion.java
---
@@ -0,0 +1,58 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.registry.extension.repo;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+import org.apache.nifi.registry.link.LinkAdapter;
+
+import javax.ws.rs.core.Link;
+import javax.xml.bind.annotation.XmlElement;
+import javax.xml.bind.annotation.XmlRootElement;
+import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
+
+@ApiModel
+@XmlRootElement
+public class ExtensionRepoVersion {
--- End diff --
these dynamically generated links worked well in my testing
---
[GitHub] nifi-registry pull request #148: NIFIREG-211 Initial work for adding extenio...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/148#discussion_r236808042
--- Diff:
nifi-registry-core/nifi-registry-data-model/src/main/java/org/apache/nifi/registry/extension/ExtensionBundleVersionMetadata.java
---
@@ -0,0 +1,161 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.registry.extension;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+import org.apache.nifi.registry.link.LinkableEntity;
+
+import javax.validation.constraints.Min;
+import javax.validation.constraints.NotBlank;
+import javax.xml.bind.annotation.XmlRootElement;
+import java.util.Objects;
+
+@ApiModel
+@XmlRootElement
+public class ExtensionBundleVersionMetadata extends LinkableEntity
implements Comparable {
+
+@NotBlank
+private String id;
+
+@NotBlank
+private String extensionBundleId;
+
+@NotBlank
+private String bucketId;
+
+@NotBlank
+private String version;
+
+private ExtensionBundleVersionDependency dependency;
+
+@Min(1)
+private long timestamp;
+
+@NotBlank
+private String author;
+
+private String description;
+
+@NotBlank
+private String sha256Hex;
--- End diff --
This is a minor nitpick, but for the JSON field name here, I would have a
slight preference for just using `sha256` as the field name as that is
consistent with the
`/extensions/repo/{bucket}/{group}/{artifact}/{version}/sha256` endpoint (and
the field description in the REST API documentation clarifies that it is the
hex digest).
---
[GitHub] nifi-registry pull request #148: NIFIREG-211 Initial work for adding extenio...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/148#discussion_r236827312
--- Diff:
nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/ExtensionRepositoryResource.java
---
@@ -0,0 +1,378 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.registry.web.api;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.Extension;
+import io.swagger.annotations.ExtensionProperty;
+import org.apache.nifi.registry.bucket.Bucket;
+import org.apache.nifi.registry.bucket.BucketItem;
+import org.apache.nifi.registry.event.EventService;
+import org.apache.nifi.registry.extension.ExtensionBundleVersion;
+import org.apache.nifi.registry.extension.repo.ExtensionRepoArtifact;
+import org.apache.nifi.registry.extension.repo.ExtensionRepoBucket;
+import org.apache.nifi.registry.extension.repo.ExtensionRepoGroup;
+import org.apache.nifi.registry.extension.repo.ExtensionRepoVersion;
+import org.apache.nifi.registry.extension.repo.ExtensionRepoVersionSummary;
+import org.apache.nifi.registry.security.authorization.RequestAction;
+import org.apache.nifi.registry.service.AuthorizationService;
+import org.apache.nifi.registry.service.RegistryService;
+import
org.apache.nifi.registry.service.extension.ExtensionBundleVersionCoordinate;
+import org.apache.nifi.registry.web.link.LinkService;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.ws.rs.Consumes;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.Link;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.StreamingOutput;
+import java.util.ArrayList;
+import java.util.Set;
+import java.util.SortedSet;
+
+@Component
+@Path("/extensions/repo")
+@Api(
+value = "extension_repository",
+description = "Interact with extension bundles via the hierarchy
of bucket/group/artifact/version.",
+authorizations = { @Authorization("Authorization") }
+)
+public class ExtensionRepositoryResource extends
AuthorizableApplicationResource {
+
+public static final String CONTENT_DISPOSITION_HEADER =
"content-disposition";
+private final RegistryService registryService;
+private final LinkService linkService;
+
+@Autowired
+public ExtensionRepositoryResource(
+final RegistryService registryService,
+final LinkService linkService,
+final AuthorizationService authorizationService,
+final EventService eventService) {
+super(authorizationService, eventService);
+this.registryService = registryService;
+this.linkService = linkService;
+}
+
+@GET
+@Consumes(MediaType.WILDCARD)
+@Produces(MediaType.APPLICATION_JSON)
+@ApiOperation(
+value = "Gets the names of the buckets the current user is
authorized for in order to browse the repo by bucket",
+response = ExtensionRepoBucket.class,
+responseContainer = "List",
+extensions = {
+@Extension(name = "access-policy", properties = {
+@ExtensionProperty(name = "action", value =
"read"),
+@ExtensionProperty(name = "resource", value =
"/buckets/{bucketId}") })
+}
--- End diff --
I don't think we want an access-policy documented here as read only access
is allowed for all authenticated users.
---
[GitHub] nifi-registry pull request #148: NIFIREG-211 Initial work for adding extenio...
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/148#discussion_r236826377 --- Diff: nifi-registry-core/nifi-registry-framework/src/main/resources/db/migration/V3__AddExtensions.sql --- @@ -0,0 +1,62 @@ +-- 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. + +CREATE TABLE EXTENSION_BUNDLE ( +ID VARCHAR(50) NOT NULL, +BUCKET_ID VARCHAR(50) NOT NULL, +BUNDLE_TYPE VARCHAR(200) NOT NULL, +GROUP_ID VARCHAR(500) NOT NULL, +ARTIFACT_ID VARCHAR(500) NOT NULL, +CONSTRAINT PK__EXTENSION_BUNDLE_ID PRIMARY KEY (ID), +CONSTRAINT FK__EXTENSION_BUNDLE_BUCKET_ITEM_ID FOREIGN KEY (ID) REFERENCES BUCKET_ITEM(ID) ON DELETE CASCADE, +CONSTRAINT FK__EXTENSION_BUNDLE_BUCKET_ID FOREIGN KEY(BUCKET_ID) REFERENCES BUCKET(ID) ON DELETE CASCADE, +CONSTRAINT UNIQUE__EXTENSION_BUNDLE_BUCKET_GROUP_ARTIFACT UNIQUE (BUCKET_ID, GROUP_ID, ARTIFACT_ID) +); + +CREATE TABLE EXTENSION_BUNDLE_VERSION ( +ID VARCHAR(50) NOT NULL, +EXTENSION_BUNDLE_ID VARCHAR(50) NOT NULL, +VERSION VARCHAR(100) NOT NULL, +DEPENDENCY_GROUP_ID VARCHAR(500), --- End diff -- CPP extensions may have more than one dependency, in which case having these in a separate table may be worthwhile. ---
[GitHub] nifi-registry pull request #148: NIFIREG-211 Initial work for adding extenio...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/148#discussion_r236837704
--- Diff:
nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/extension/StandardExtensionService.java
---
@@ -0,0 +1,589 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.registry.service.extension;
+
+import org.apache.commons.codec.binary.Hex;
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Validate;
+import org.apache.nifi.registry.bucket.Bucket;
+import org.apache.nifi.registry.db.entity.BucketEntity;
+import org.apache.nifi.registry.db.entity.ExtensionBundleEntity;
+import org.apache.nifi.registry.db.entity.ExtensionBundleEntityType;
+import org.apache.nifi.registry.db.entity.ExtensionBundleVersionEntity;
+import org.apache.nifi.registry.exception.ResourceNotFoundException;
+import org.apache.nifi.registry.extension.BundleCoordinate;
+import org.apache.nifi.registry.extension.BundleDetails;
+import org.apache.nifi.registry.extension.BundleExtractor;
+import org.apache.nifi.registry.extension.ExtensionBundle;
+import org.apache.nifi.registry.extension.ExtensionBundleContext;
+import
org.apache.nifi.registry.extension.ExtensionBundlePersistenceProvider;
+import org.apache.nifi.registry.extension.ExtensionBundleType;
+import org.apache.nifi.registry.extension.ExtensionBundleVersion;
+import org.apache.nifi.registry.extension.ExtensionBundleVersionDependency;
+import org.apache.nifi.registry.extension.ExtensionBundleVersionMetadata;
+import org.apache.nifi.registry.extension.repo.ExtensionRepoArtifact;
+import org.apache.nifi.registry.extension.repo.ExtensionRepoBucket;
+import org.apache.nifi.registry.extension.repo.ExtensionRepoGroup;
+import org.apache.nifi.registry.extension.repo.ExtensionRepoVersionSummary;
+import org.apache.nifi.registry.properties.NiFiRegistryProperties;
+import
org.apache.nifi.registry.provider.extension.StandardExtensionBundleContext;
+import org.apache.nifi.registry.security.authorization.user.NiFiUserUtils;
+import org.apache.nifi.registry.service.DataModelMapper;
+import org.apache.nifi.registry.service.MetadataService;
+import org.apache.nifi.registry.util.FileUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.ConstraintViolationException;
+import javax.validation.Validator;
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.security.DigestInputStream;
+import java.security.MessageDigest;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.UUID;
+import java.util.stream.Collectors;
+
+@Service
+public class StandardExtensionService implements ExtensionService {
+
+private static final Logger LOGGER =
LoggerFactory.getLogger(StandardExtensionService.class);
+
+private final MetadataService metadataService;
+private final Map extractors;
+private final ExtensionBundlePersistenceProvider
bundlePersistenceProvider;
+private final Validator validator;
+private final File extensionsWorkingDir;
+
+@Autowired
+public StandardExtensionService(final MetadataService metadataService,
+final Map extractors,
+final
ExtensionBundl
[GitHub] nifi-minifi-cpp issue #438: MINIFICPP-675: Fix issue with hearder evaluation...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-minifi-cpp/pull/438 Thanks a lot for the quick turn around on this @phrocker! ---
[GitHub] nifi issue #3129: NIFI-5748 Fixed proxy header support to use X-Forwarded-Ho...
Github user kevdoran commented on the issue: https://github.com/apache/nifi/pull/3129 Thanks, @jtstorck! Will review... ---
[GitHub] nifi-registry issue #144: NIFIREG-209 Rebuild metadata DB from FlowPersisten...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/144 This is a fun new feature @bbende. It will be nice for DR or as a mechanism to populate NiFi Registry instances in new environments with some canned flows. I looked at the code and tried it out and it's working well for me. There is some oddness when loading flows from 0.2.0 or 0.3.0 as they don't have all the metadata available, but I think that's fine. Going forward with git repos created after this feature exists should have everything there. Interested to hear from @ijokarumawak on this to get his input as he is the most experienced with the Java Git stuff, but consider me a tentative +1 on this PR. Cool stuff! ---
[GitHub] nifi issue #3043: NIFI-5656 Remove "Node Group" from the default FileAccessP...
Github user kevdoran commented on the issue: https://github.com/apache/nifi/pull/3043 +1 thanks! Merged to master... ---
[GitHub] nifi issue #3043: NIFI-5656 Remove "Node Group" from the default FileAccessP...
Github user kevdoran commented on the issue: https://github.com/apache/nifi/pull/3043 Thansk @pepov. I had one minor comment regarding logging, but aside from that these changes look good to me ---
[GitHub] nifi pull request #3043: NIFI-5656 Remove "Node Group" from the default File...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi/pull/3043#discussion_r222308394
--- Diff:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAccessPolicyProvider.java
---
@@ -232,16 +232,21 @@ public void
onConfigured(AuthorizerConfigurationContext configurationContext) th
nodeGroupIdentifier = null;
if (nodeGroupName != null) {
-for (Group group : userGroupProvider.getGroups()) {
-if (group.getName().equals(nodeGroupName)) {
-nodeGroupIdentifier = group.getIdentifier();
-break;
+if (!StringUtils.isBlank(nodeGroupName)) {
+logger.debug("Trying to load node group '{}' from the
underlying userGroupProvider", nodeGroupName);
+for (Group group : userGroupProvider.getGroups()) {
+if (group.getName().equals(nodeGroupName)) {
+nodeGroupIdentifier = group.getIdentifier();
+break;
+}
}
-}
-if (nodeGroupIdentifier == null) {
-throw new AuthorizerCreationException(String.format(
+if (nodeGroupIdentifier == null) {
+throw new
AuthorizerCreationException(String.format(
"Authorizations node group '%s' could not be
found", nodeGroupName));
+}
+} else {
+logger.warn("Empty node group name provided");
--- End diff --
I don't think this merits a warning, given that it empty by default.
Perhaps a debug message would be appropriate.
---
[GitHub] nifi issue #3043: NIFI-5656 Remove "Node Group" from the default FileAccessP...
Github user kevdoran commented on the issue: https://github.com/apache/nifi/pull/3043 Thanks @pepov! I'll re-review the PR when your changes are in. ---
[GitHub] nifi-registry issue #143: NIFIREG-201 Refactoring project structure to bette...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/143 Will review... ---
[GitHub] nifi-registry issue #142: NIFIREG-200 Update dependencies
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/142 Good catch @alopresto. I was simply trying to pin a single version of Guava as we were using 18.0 but 17.0 was being pulled in transitively. But I don't see any reason not to pin version 26.0, so I updated my branch to use that. Compilation and runtime seem unaffected. Thanks! ---
[GitHub] nifi-registry issue #142: NIFIREG-200 Update dependencies
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/142 @bbende if you could review this when you get a chance that would be appreciated ---
[GitHub] nifi-registry pull request #142: NIFIREG-200 Update dependencies
GitHub user kevdoran opened a pull request: https://github.com/apache/nifi-registry/pull/142 NIFIREG-200 Update dependencies - Update Jetty to version 9.4.11.v20180605 - Update Spring Boot to version 2.0.4.RELEASE - Update Spring Security to version 5.0.7.RELEASE - Update Jackson to version 2.9.6 - Fix guava to version 18.0 (mutliple versions were being pulled in previously) You can merge this pull request into a Git repository by running: $ git pull https://github.com/kevdoran/nifi-registry NIFIREG-200-dep-version-bump Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi-registry/pull/142.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 #142 commit 66a9e07c8d2166e379eebc358b03bcf960700eee Author: Kevin Doran Date: 2018-09-21T00:01:49Z NIFIREG-200 Update dependencies - Update Jetty to version 9.4.11.v20180605 - Update Spring Boot to version 2.0.4.RELEASE - Update Spring Security to version 5.0.7.RELEASE - Update Jackson to version 2.9.6 - Fix guava to version 18.0 (mutliple versions were being pulled in previously) ---
[GitHub] nifi-registry pull request #131: NIFIREG-186: Adding Ranger authorizer
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/131#discussion_r219256607 --- Diff: nifi-registry-extensions/nifi-registry-ranger/nifi-registry-ranger-assembly/README.md --- @@ -0,0 +1,131 @@ + +# NiFi Registry Ranger extension + +This extension provides `org.apache.nifi.registry.ranger.RangerAuthorizer` class for NiFi Registry to authorize user requests by access policies defined at [Apache Ranger](https://ranger.apache.org/). + +## Prerequisites + +* Apache Ranger 1.2.0 or later is needed. --- End diff -- Ok, looks like this is good for Ranger 1.2.0 and 2.0.0. I'll merge it as-is then, thanks! ---
[GitHub] nifi-registry pull request #131: NIFIREG-186: Adding Ranger authorizer
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/131#discussion_r218507868 --- Diff: nifi-registry-extensions/nifi-registry-ranger/nifi-registry-ranger-assembly/README.md --- @@ -0,0 +1,131 @@ + +# NiFi Registry Ranger extension + +This extension provides `org.apache.nifi.registry.ranger.RangerAuthorizer` class for NiFi Registry to authorize user requests by access policies defined at [Apache Ranger](https://ranger.apache.org/). + +## Prerequisites + +* Apache Ranger 1.2.0 or later is needed. --- End diff -- Is this correct? Based on the changes submitted to Apache Ranger that will add support for this, it looks like it will require Apache Ranger >= 2.0.0, correct? ---
[GitHub] nifi-registry issue #140: NIFIREG-199 - Adding interfaces to represent confi...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/140 Reviewing... ---
[GitHub] nifi pull request #2983: NIFI-5566 Improve HashContent processor and standar...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2983#discussion_r218085671
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/groovy/org/apache/nifi/security/util/crypto/HashServiceTest.groovy
---
@@ -0,0 +1,457 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.security.util.crypto
+
+import org.apache.nifi.components.AllowableValue
+import org.bouncycastle.jce.provider.BouncyCastleProvider
+import org.bouncycastle.util.encoders.Hex
+import org.junit.After
+import org.junit.AfterClass
+import org.junit.Before
+import org.junit.BeforeClass
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+import java.nio.charset.Charset
+import java.nio.charset.StandardCharsets
+import java.security.Security
+
+@RunWith(JUnit4.class)
+class HashServiceTest extends GroovyTestCase {
+private static final Logger logger =
LoggerFactory.getLogger(HashServiceTest.class)
+static private final String LARGE_FILE_PATH =
"src/test/resources/HashServiceTest/largefile.txt"
--- End diff --
This is minor, but this path could be changed to be located in a sub-dir of
`target` instead of `src`, which would have the following advantages:
- would not need to be generated every time the tests are run and would not
need to be manually removed, as it be destroyed only on a `mvn clean`
- would not show up as a git untracked file, as everything under `target`
is ignored
- would not be scanned but the Apache RAT plugin (IIRC, it does not scan
`target`?)
- would not require the `.keep` file to maintain a directory if you put it
in a directory that you expect the build to generate, ie `target/test-classes`
I know that most of these are mitigated by the removal of the file in the
`tearDownOnce()` method, but should the execution / JVM halt for any reason
prior to that method executing (could be more likely in a multi-threaded build
due to errors in other tests), then there is a chance that the generated file
remains in the src directory and could create confusion / unintended
consequences if followed by something like a `git add -A` command.
Like I said, this is a minor nitpick, not a blocker by any means :)
---
[GitHub] nifi-registry pull request #139: NIFIREG-198 Fix VersionedRemoteProcessGroup...
GitHub user kevdoran opened a pull request: https://github.com/apache/nifi-registry/pull/139 NIFIREG-198 Fix VersionedRemoteProcessGroup targetUri bug VersionedRemoteProcessGroup has two fields: targetUri and targetUris. They do not have simple getters, but rather each will fallback to using the other. Given how they are described, for certain combinations of targetUri and targetUris, they do not work correctly (mainly, when targetUri is not set and targetUris contains only a single value). Furthermore, in NiFi only TargetUris is used/needed. It was a replacement for targetUri when the requirement for multiple URIs was added. Therefore, in addition to fixing the bug descirbed above, targetUri can be deprecated and dropped in future versions of NiFi Registry. You can merge this pull request into a Git repository by running: $ git pull https://github.com/kevdoran/nifi-registry NIFIREG-198-vrpg-targetUri Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi-registry/pull/139.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 #139 commit 79af0dbe771e14b3e38e713c0ae9d860a7e83eb2 Author: Kevin Doran Date: 2018-09-06T23:10:50Z NIFIREG-198 Add unit test that reproduces bug commit bc138f1d8649eba3a7daf4ef8acd3e2e2f733622 Author: Kevin Doran Date: 2018-09-07T00:26:06Z NIFIREG-198 Fix VersionedRemoteProcesGroup targetUri commit 73ac36f678a3d82acdc935e18c5b0cf08b4064f0 Author: Kevin Doran Date: 2018-09-07T14:53:09Z NIFIREG-198 Deprecate VersionedRemoteProcessGroup targetUri ---
[GitHub] nifi issue #2970: NIFI-5542 Added support for node groups to FileAccessPolic...
Github user kevdoran commented on the issue: https://github.com/apache/nifi/pull/2970 Yes, that should work fine to lookup group by name. Good point regarding the field name of the Group object. Thanks for looking into this! ---
[GitHub] nifi issue #2970: NIFI-5542 Added support for node groups to FileAccessPolic...
Github user kevdoran commented on the issue: https://github.com/apache/nifi/pull/2970 Ok, thanks @pepov -- that clears up the intention of these changes to me. For naming of variables/properties I'm good with either _identity_ or _name_, as you suggest - both are clear to me and distinct from _identifier_. I was more concerned with the usage of the method `UserGroupProvider.getGroup(String groupIdentifier)`, which will will have to change if the value is not actually an identifier. For accessing users, the [UserGroupProvider](https://github.com/apache/nifi/blob/master/nifi-framework-api/src/main/java/org/apache/nifi/authorization/UserGroupProvider.java) interface provides both `getUser(String identifier)` and `getUserByIdentity(String identity)` methods. I don't see an equivalent method for groups that is a lookup by name/identity, so you'll have to work around that. ---
[GitHub] nifi pull request #2970: NIFI-5542 Added support for node groups to FileAcce...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2970#discussion_r213438246
--- Diff:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAccessPolicyProvider.java
---
@@ -114,6 +114,7 @@ private static JAXBContext initializeJaxbContext(final
String contextPath) {
static final String WRITE_CODE = "W";
static final String PROP_NODE_IDENTITY_PREFIX = "Node Identity ";
+static final String PROP_NODE_GROUP = "Node Group";
--- End diff --
The [Admin
Guide](https://github.com/apache/nifi/blob/master/nifi-docs/src/main/asciidoc/administration-guide.adoc#fileaccesspolicyprovider)
and default
[authorizers.xml](https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/conf/authorizers.xml)
file should be updated to include this property and its usage.
---
[GitHub] nifi-registry issue #134: NIFIREG-192: Implement REGISTRY_START event
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/134 +1, looks good and verified event is triggered when expected. Thanks for the contribution @jdye64! ---
[GitHub] nifi-registry issue #135: [NIFIREG-193] upgrade superagent
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/135 @scottyaslan corrected with an [empty commit](https://github.com/apache/nifi-registry/commit/ead0ea6e66f7bba21c65769bacc387beaf1a185a). ---
[GitHub] nifi-registry issue #134: NIFIREG-192: Implement REGISTRY_START event
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/134 @jdye64 Thanks for that change. Now that #133 is merged, could you updated the documentation to add your new `REGISTRY_START` event type to the _Whitelisted Event Type x_ property value table? ---
[GitHub] nifi-registry issue #131: NIFIREG-186: Adding Ranger authorizer
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/131 @ijokarumawak Thanks for the update. I'll review the most recent commits sometime over the next few days and test with those additional configurations. I'll watch the Ranger PR that is still under review, and help get this PR merged once that is finalized. Let me know if I can help to get either pull request finalized, reviewed, or tested. ---
[GitHub] nifi-registry issue #131: NIFIREG-186: Adding Ranger authorizer
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/131 > The improvement at Ranger side is under review, but not merged yet. I think we can wait until Ranger side gets merged. In the mean while, let's confirm it works with Kerberos and HDF audit. Sounds good. Iâll have limited availability for the next week or so, but will verify those configurations As soon as possible. Thanks! ---
[GitHub] nifi-registry issue #133: NIFIREG-190
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/133 A couple of other things I thought of: - The default [providers.xml](https://github.com/apache/nifi-registry/blob/7e4e9713406f3bb3ac1544b6ad69fbdd437f794c/nifi-registry-resources/src/main/resources/conf/providers.xml#L34) should maybe be updated to include placeholders/explanation for the whitelist properties. - By adding the `shouldHandle(EventType)` method to the EventHookProvider interface, should we move the conditional execution of the hook's `handle(Event)` method to a check where it is called in [EventService](https://github.com/apache/nifi-registry/blob/7e4e9713406f3bb3ac1544b6ad69fbdd437f794c/nifi-registry-framework/src/main/java/org/apache/nifi/registry/event/EventService.java#L73)? ---
[GitHub] nifi-registry pull request #133: NIFIREG-190
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/133#discussion_r209136065 --- Diff: nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/hook/AbstractEventHookProvider.java --- @@ -0,0 +1,72 @@ +/* + * 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. + */ +package org.apache.nifi.registry.provider.hook; + +import org.apache.commons.lang3.StringUtils; +import org.apache.nifi.registry.hook.EventHookProvider; +import org.apache.nifi.registry.hook.EventType; +import org.apache.nifi.registry.provider.ProviderConfigurationContext; +import org.apache.nifi.registry.provider.ProviderCreationException; +import org.springframework.util.CollectionUtils; + +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public abstract class AbstractEventHookProvider --- End diff -- I'm good with either. If we move it to the API, so third-party impls can use it (which would be nice), we should probably remove the dep on `org.springframework.util.CollectionUtils` ---
[GitHub] nifi-registry pull request #133: NIFIREG-190
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/133#discussion_r209032885 --- Diff: nifi-registry-provider-api/src/main/java/org/apache/nifi/registry/hook/EventHookProvider.java --- @@ -36,4 +36,18 @@ */ void handle(Event event) throws EventHookException; +/** + * Examines the values from the 'Event Whitelist X' properties in the hook provider definition to determine + * if the Event should be invoked for this particular EventType --- End diff -- 'Event Whitelist X' property has been renamed in this commit to 'Whitelisted Event Type '. ---
[GitHub] nifi-registry pull request #134: NIFIREG-192: Implement REGISTRY_START event
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/134#discussion_r208977230
--- Diff:
nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/NiFiRegistryResourceConfig.java
---
@@ -44,6 +48,14 @@
private static final Logger logger =
LoggerFactory.getLogger(NiFiRegistryResourceConfig.class);
+@Autowired
+private StandardProviderFactory standardProviderFactory;
+
+@Bean
+public EventService eventService() {
--- End diff --
I don't think this is required because `EventService` is already annotated
with `@Service`, so it will be created as a bean by component scanning.
---
[GitHub] nifi-registry pull request #134: NIFIREG-192: Implement REGISTRY_START event
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/134#discussion_r208965183
--- Diff:
nifi-registry-web-api/src/main/java/org/apache/nifi/registry/NiFiRegistryApiApplication.java
---
@@ -54,6 +63,15 @@ protected SpringApplicationBuilder
configure(SpringApplicationBuilder applicatio
.properties(defaultProperties);
}
+@PostConstruct
+public void postConstruct() {
--- End diff --
I'm not sure when post construct gets called for a
SpringApplicationBuilder. I don't think we want to fire the REGISTRY_START
event until after the full application context is loaded, otherwise
EventHookProvider consumers of that event type might not be loaded/ready yet.
Take a look at the `ApplicationListener` interface,
that could be a nice solution to this.
---
[GitHub] nifi-registry issue #133: NIFIREG-190
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/133 Thanks @jdye64! That sounds good, I'll give it another look when those changes are ready. ---
[GitHub] nifi-registry pull request #133: NIFIREG-190
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/133#discussion_r208955763
--- Diff:
nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/hook/AbstractHookProvider.java
---
@@ -0,0 +1,70 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.registry.provider.hook;
+
+import org.apache.nifi.registry.hook.Event;
+import org.apache.nifi.registry.hook.EventHookException;
+import org.apache.nifi.registry.hook.EventHookProvider;
+import org.apache.nifi.registry.hook.EventType;
+import org.apache.nifi.registry.provider.ProviderConfigurationContext;
+import org.apache.nifi.registry.provider.ProviderCreationException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.util.CollectionUtils;
+
+import java.util.List;
+
+/**
+ * An AbstractHookProvider class that serves as a place to keep common
Event Hook implementation details.
+ */
+public abstract class AbstractHookProvider
+implements EventHookProvider {
+
+static final Logger LOGGER =
LoggerFactory.getLogger(AbstractHookProvider.class);
+
+static final String EVENT_WHITELIST = "Event Whitelist";
+protected List whiteListEvents = null;
--- End diff --
This would be better as `Set` so that `contains()` lookups are
O(1)
---
[GitHub] nifi-registry pull request #133: NIFIREG-190
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/133#discussion_r208955444
--- Diff:
nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/hook/LoggingEventHookProvider.java
---
@@ -36,6 +36,10 @@ public void onConfigured(final
ProviderConfigurationContext configurationContext
@Override
public void handle(final Event event) throws EventHookException {
+
+// Purposely leaving off "handleEvent" logic as its assumed that
everything should be logged.
+//handleEvent(event)
--- End diff --
but that contradicts the documentation of the shared property... again I
think we need to discuss what the intention is and then implement the code to
match
---
[GitHub] nifi-registry pull request #133: NIFIREG-190
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/133#discussion_r208955153
--- Diff:
nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/hook/AbstractHookProvider.java
---
@@ -0,0 +1,70 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.registry.provider.hook;
+
+import org.apache.nifi.registry.hook.Event;
+import org.apache.nifi.registry.hook.EventHookException;
+import org.apache.nifi.registry.hook.EventHookProvider;
+import org.apache.nifi.registry.hook.EventType;
+import org.apache.nifi.registry.provider.ProviderConfigurationContext;
+import org.apache.nifi.registry.provider.ProviderCreationException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.util.CollectionUtils;
+
+import java.util.List;
+
+/**
+ * An AbstractHookProvider class that serves as a place to keep common
Event Hook implementation details.
+ */
+public abstract class AbstractHookProvider
+implements EventHookProvider {
+
+static final Logger LOGGER =
LoggerFactory.getLogger(AbstractHookProvider.class);
+
+static final String EVENT_WHITELIST = "Event Whitelist";
+protected List whiteListEvents = null;
+
+@Override
+public void handle(Event event) throws EventHookException {
+// Abstract event handling logic can be implemented here.
+}
+
+@Override
+public void onConfigured(ProviderConfigurationContext
configurationContext) throws ProviderCreationException {
+
+}
--- End diff --
This is an abstract class so we don't need the empty interface methods for
`handle(...)` and `onConfigured(...)`
---
[GitHub] nifi-registry pull request #133: NIFIREG-190
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/133#discussion_r208954806
--- Diff:
nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/hook/AbstractHookProvider.java
---
@@ -0,0 +1,70 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.registry.provider.hook;
+
+import org.apache.nifi.registry.hook.Event;
+import org.apache.nifi.registry.hook.EventHookException;
+import org.apache.nifi.registry.hook.EventHookProvider;
+import org.apache.nifi.registry.hook.EventType;
+import org.apache.nifi.registry.provider.ProviderConfigurationContext;
+import org.apache.nifi.registry.provider.ProviderCreationException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.util.CollectionUtils;
+
+import java.util.List;
+
+/**
+ * An AbstractHookProvider class that serves as a place to keep common
Event Hook implementation details.
+ */
+public abstract class AbstractHookProvider
+implements EventHookProvider {
+
+static final Logger LOGGER =
LoggerFactory.getLogger(AbstractHookProvider.class);
+
+static final String EVENT_WHITELIST = "Event Whitelist";
+protected List whiteListEvents = null;
+
+@Override
+public void handle(Event event) throws EventHookException {
+// Abstract event handling logic can be implemented here.
+}
+
+@Override
+public void onConfigured(ProviderConfigurationContext
configurationContext) throws ProviderCreationException {
+
+}
+
+/**
+ * Determines if the Event should be handled or ignored based on the
user configured Event Whitelist property
+ *
+ * @param event
+ * Event that has been passed to the provider by the framework.
+ *
+ * @return
+ * True if the event should be handled by this provider and false
otherwise.
+ */
+public boolean handleEvent(Event event) {
--- End diff --
As commented on the PR, this should perhaps be implemented differently. But
if going with the Abstract base class approach, I would change the signature of
this method to:
```
protected boolean shouldHandle(Event event)
// or
protected boolean shouldHandle(EventType eventType)
```
---
[GitHub] nifi-registry pull request #133: NIFIREG-190
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/133#discussion_r208953844 --- Diff: nifi-registry-docs/src/main/asciidoc/administration-guide.adoc --- @@ -1094,3 +1094,63 @@ Here is the data model version histories: |2|0.2|JSON formatted text file. The root object contains header and Flow content object. |1|0.1|Binary format having header bytes at the beginning followed by Flow content represented as XML. | + +== Event Hooks +Event hooks are an integration point that allows for custom code to to be triggered when NiFi Registry application events occur. + +[options="header,footer"] +|== +| Event Name | Description +|`CREATE_BUCKET` | A new registry bucket is created. +|`CREATE_FLOW` | A new flow is created in a specified bucket. Only triggered on first time creation of a flow with a given name. +|`CREATE_FLOW_VERSION` | A new version for a flow has been saved in the registry. +|`UPDATE_BUCKET` | A bucket has been updated. +|`UPDATE_FLOW` | A flow that exist in a bucket has been updated. +|`DELETE_BUCKET` | An existing bucket in the registry is deleted. +|`DELETE_FLOW` | An existing flow in the registry is deleted. +|== + +=== Shared Event Hook Properties +There are certain properties that are shared amongst all of the NiFi Registry provided Event Hook implementations. Those properties and +their purpose are listed below. + +[options="header,footer"] +|== +| Property Name | Description +|`Event Whitelist` | A comma separated list of events the the hook provider configured with this property should respond to. If this property is left blank or not provided all events will fire for the configured hook provider. +|== + +=== ScriptEventHookProvider +Hook provider for invoking a shell script that has been written by a user and placed on a file system that is accessible +by the NiFi Registry instance that the provider is configured for. + + + + + org.apache.nifi.registry.provider.hook.ScriptEventHookProvider + + + + +CREATE_FLOW,UPDATE_FLOW --- End diff -- Comma-separated lists in XML is inconsistent with how we handle configuring lists in other contexts. For example, the initial user identities in Authorizers.xml: ``` file-user-group-provider org.apache.nifi.registry.security.authorization.file.FileUserGroupProvider ./conf/users.xml ``` We may want to reconsider in order to keep this consistent ---
[GitHub] nifi-registry issue #133: NIFIREG-190
Github user kevdoran commented on the issue:
https://github.com/apache/nifi-registry/pull/133
If we are allowing each implementation to decide the logic in the handle
method, I don't see the value in the "share property" concept as documented:
> There are certain properties that are shared amongst all of the NiFi
Registry provided Event Hook implementations.
I don't think we should introduce the concept of a shared property that is
documented as being honored by all implementations when we really have no way
to guarantee that.
In this PR, the implementation that does handle the Event Whitelist is
implemented as an Abstract base class in the framework, which would make it
difficult for third-party extension providers to implement (or they could
ignore it if they want as it is not part of the API).
If we want to make it universal, I think we would need to introduce a
dedicated field for whitelist that is enforced by the framework code (perhaps
using a decorator / wrapper), not the extension. Otherwise, don't think we make
it universal at all, it can just be an optional thing that third parties can
choose to implement.
One way to make it more obvious to custom implementations that this is
intended to be implemented would be to make it part of the interface they have
to implement (otherwise it is just documentation and hard to enforce). For
example, we could add a new method with a default implementation to the
`EventHookProvider` interface. For example:
```
default boolean shouldHandle(EventType eventType) {
return true;
}
```
If it's not overridden with a custom implementation, the event hook
provider will always be called. If it is overridden, they can return a boolean
for the event types they are interested in, either as a hard-coded set or a
user-configurable set (they would still need to document their own
configuration property for that).
---
[GitHub] nifi-registry issue #133: NIFIREG-190
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/133 Will review... ---
[GitHub] nifi-registry issue #131: NIFIREG-186: Adding Ranger authorizer
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/131 Thanks @ijokarumawak! I'll finish up my review based on your latest changes & comments. ---
[GitHub] nifi-registry pull request #131: NIFIREG-186: Adding Ranger authorizer
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/131#discussion_r204467213
--- Diff:
nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/ConfigResource.java
---
@@ -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.
+ */
+package org.apache.nifi.registry.web.api;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.Extension;
+import io.swagger.annotations.ExtensionProperty;
+import org.apache.nifi.registry.RegistryConfiguration;
+import org.apache.nifi.registry.event.EventService;
+import org.apache.nifi.registry.security.authorization.Authorizer;
+import
org.apache.nifi.registry.security.authorization.AuthorizerCapabilityDetection;
+import org.apache.nifi.registry.security.authorization.RequestAction;
+import
org.apache.nifi.registry.security.authorization.exception.AccessDeniedException;
+import
org.apache.nifi.registry.security.authorization.resource.Authorizable;
+import org.apache.nifi.registry.service.AuthorizationService;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.ws.rs.Consumes;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+@Component
+@Path("/config")
+@Api(
+value = "config",
+description = "Retrieves the configuration for this NiFi
Registry.",
+authorizations = { @Authorization("Authorization") }
+)
+public class ConfigResource extends AuthorizableApplicationResource {
+
+@Autowired
+public ConfigResource(
+final AuthorizationService authorizationService,
+final EventService eventService) {
+super(authorizationService, eventService);
+}
+
+@GET
+@Consumes(MediaType.WILDCARD)
+@Produces(MediaType.APPLICATION_JSON)
+@ApiOperation(
+value = "Gets NiFi Registry configurations",
+response = RegistryConfiguration.class,
+extensions = {
+@Extension(name = "access-policy", properties = {
+@ExtensionProperty(name = "action", value =
"read"),
+@ExtensionProperty(name = "resource", value =
"/config") })
--- End diff --
This is minor as it only affects REST API documentation, but `/config` is
not an authorizable
[ResourceType](https://github.com/apache/nifi-registry/blob/master/nifi-registry-framework/src/main/java/org/apache/nifi/registry/security/authorization/resource/ResourceType.java).
I would change this line to include the two resource paths that are being used
to authorize the request, so that those are reflected in the documentation:
@ExtensionProperty(name = "resource", value = "/policies,/tenants") })
---
[GitHub] nifi-registry pull request #131: NIFIREG-186: Adding Ranger authorizer
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/131#discussion_r204468072
--- Diff:
nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/ConfigResource.java
---
@@ -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.
+ */
+package org.apache.nifi.registry.web.api;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.Extension;
+import io.swagger.annotations.ExtensionProperty;
+import org.apache.nifi.registry.RegistryConfiguration;
+import org.apache.nifi.registry.event.EventService;
+import org.apache.nifi.registry.security.authorization.Authorizer;
+import
org.apache.nifi.registry.security.authorization.AuthorizerCapabilityDetection;
+import org.apache.nifi.registry.security.authorization.RequestAction;
+import
org.apache.nifi.registry.security.authorization.exception.AccessDeniedException;
+import
org.apache.nifi.registry.security.authorization.resource.Authorizable;
+import org.apache.nifi.registry.service.AuthorizationService;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.ws.rs.Consumes;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+@Component
+@Path("/config")
+@Api(
+value = "config",
+description = "Retrieves the configuration for this NiFi
Registry.",
+authorizations = { @Authorization("Authorization") }
+)
+public class ConfigResource extends AuthorizableApplicationResource {
+
+@Autowired
+public ConfigResource(
+final AuthorizationService authorizationService,
+final EventService eventService) {
+super(authorizationService, eventService);
+}
+
+@GET
+@Consumes(MediaType.WILDCARD)
+@Produces(MediaType.APPLICATION_JSON)
+@ApiOperation(
+value = "Gets NiFi Registry configurations",
+response = RegistryConfiguration.class,
+extensions = {
+@Extension(name = "access-policy", properties = {
+@ExtensionProperty(name = "action", value =
"read"),
+@ExtensionProperty(name = "resource", value =
"/config") })
+}
+)
+@ApiResponses({ @ApiResponse(code = 401, message =
HttpStatusMessages.MESSAGE_401) })
--- End diff --
Again, just a documentation issue, but as the body of the method includes
an authorization check, this should include the possibility of a 403 response.
That is:
@ApiResponses({
@ApiResponse(code = 401, message = HttpStatusMessages.MESSAGE_401),
@ApiResponse(code = 403, message = HttpStatusMessages.MESSAGE_403)
})
---
[GitHub] nifi-registry pull request #131: NIFIREG-186: Adding Ranger authorizer
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/131#discussion_r204477299 --- Diff: nifi-registry-ranger/src/test/resources/ranger/ranger-nifi-registry-security.xml --- @@ -0,0 +1,84 @@ + + + + + + ranger.plugin.nifi-registry.policy.rest.url + + http://192.168.99.100:6080 --- End diff -- Does the _TODO_ comment here apply to this PR? ---
[GitHub] nifi-minifi pull request #130: MINIFI-441 Update Docker configuration and do...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/130#discussion_r197978101
--- Diff: minifi-docker/dockerhub/Dockerfile ---
@@ -34,6 +34,8 @@ RUN mkdir -p $MINIFI_HOME
RUN apk --no-cache add curl
+ADD sh/ ${MINIFI_BASE_DIR}/scripts/
--- End diff --
I was curious and looked it up as well. I found the correct answer for
Dockerfile syntax here:
https://docs.docker.com/engine/reference/builder/#environment-replacement
> Environment variables are notated in the Dockerfile either with
$variable_name or ${variable_name}. They are treated equivalently and the brace
syntax is typically used to address issues with variable names with no
whitespace, like ${foo}_bar
Good eye, and I agree it's always better to point something out on reviews.
Everyone sees things slightly differently, and even if "correct" functionally,
it may not be the author's intention. Thanks!
---
[GitHub] nifi-minifi pull request #130: MINIFI-441 Update Docker configuration and do...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/130#discussion_r197976141
--- Diff: minifi-docker/dockerhub/Dockerfile ---
@@ -34,6 +34,8 @@ RUN mkdir -p $MINIFI_HOME
RUN apk --no-cache add curl
+ADD sh/ ${MINIFI_BASE_DIR}/scripts/
--- End diff --
Fair point. It also occurred to me later that I'm not sure if Dockerfile
syntax has any special interpretation / side-effect that differs for POSIX...
of that I'm not positive but in any case it seems to work ok :)
---
[GitHub] nifi-minifi pull request #130: MINIFI-441 Update Docker configuration and do...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/130#discussion_r197971051
--- Diff: minifi-docker/dockerhub/Dockerfile ---
@@ -34,6 +34,8 @@ RUN mkdir -p $MINIFI_HOME
RUN apk --no-cache add curl
+ADD sh/ ${MINIFI_BASE_DIR}/scripts/
--- End diff --
Either format is valid, and in this case, equivalent.
From the [POSIX
spec](http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02):
> The simplest form for parameter expansion is:
>
>${parameter}
>
> The value, if any, of parameter shall be substituted.
>
> The parameter name or symbol can be enclosed in braces, which are
optional... If the parameter is not enclosed in braces, and is a name, the
expansion shall use the longest valid name
---
[GitHub] nifi-registry issue #119: NIFIREG-172 Adds Swagger UI
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/119 @bbende I updated the copyright date range and also made a couple other updates to the README files while I was there. Let me know if you are good with these changes. Thanks! ---
[GitHub] nifi-registry issue #124: NIFIREG-174 Fixing start-up to look for the system...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/124 reviewing... ---
[GitHub] nifi-registry issue #119: NIFIREG-172 Adds Swagger UI
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/119 Good eye, will update. And thanks for the review! ---
[GitHub] nifi-registry issue #122: NIFIREG-173 Improving logic for detecting existenc...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/122 Reviewing... ---
[GitHub] nifi-registry pull request #121: NIFIREG-173 Refactor metadata DB to be inde...
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/121#discussion_r192790706 --- Diff: nifi-registry-docs/src/main/asciidoc/administration-guide.adoc --- @@ -867,12 +867,32 @@ content of the flows saved to the registry. For further details on persistence p These properties define the settings for the Registry database, which keeps track of metadata about buckets and all items stored in buckets. +The 0.1.0 release leveraged an embedded H2 database that was configured via the following properties: + | |*Property*|*Description* |nifi.registry.db.directory|The location of the Registry database directory. The default value is `./database`. |nifi.registry.db.url.append|This property specifies additional arguments to add to the connection string for the Registry database. The default value should be used and should not be changed. It is: `;LOCK_TIMEOUT=25000;WRITE_DELAY=0;AUTO_SERVER=FALSE`. | +The 0.2.0 release introduced a more flexible approach which allows leveraging an external database. This new approach +is configured via the following properties: --- End diff -- Good writeup. Clear and concise. ---
[GitHub] nifi-registry pull request #121: NIFIREG-173 Refactor metadata DB to be inde...
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi-registry/pull/121#discussion_r192790400 --- Diff: nifi-registry-framework/src/main/resources/db/migration/V2__Initial.sql --- @@ -0,0 +1,58 @@ +-- 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. + --- End diff -- I tried this schema in H2, Postgres and MySQL. For the most part looks good. I did get this error in MySQL: ``` Column length too big for column 'DESCRIPTION' (max = 21845); use BLOB or TEXT instead ``` I did a bit of research and this was the best explanation I found: https://dev.mysql.com/doc/refman/5.7/en/column-count-limit.html Seems like 65535 should be fine so long as the total row size stays below that limit. Maybe the 21845 value in the error message is due to a text encoding type (did not dig into it past that). In any case, to maintain mysql compatibility, you may want to lower those max length sizes or change the description fields to TEXT as I don't anticipate we will need to search on those. Aside from that, this schema looks good to me as a clean starting point for the new database. Nice work. ---
[GitHub] nifi-registry pull request #121: NIFIREG-173 Refactor metadata DB to be inde...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/121#discussion_r192781546
--- Diff:
nifi-registry-framework/src/main/java/org/apache/nifi/registry/db/CustomFlywayMigrationStrategy.java
---
@@ -0,0 +1,147 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.registry.db;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.registry.db.migration.BucketEntityV1;
+import org.apache.nifi.registry.db.migration.FlowEntityV1;
+import org.apache.nifi.registry.db.migration.FlowSnapshotEntityV1;
+import org.apache.nifi.registry.db.migration.LegacyDataSourceFactory;
+import org.apache.nifi.registry.db.migration.LegacyDatabaseService;
+import org.apache.nifi.registry.db.migration.LegacyEntityMapper;
+import org.apache.nifi.registry.properties.NiFiRegistryProperties;
+import org.apache.nifi.registry.service.MetadataService;
+import org.flywaydb.core.Flyway;
+import org.flywaydb.core.api.FlywayException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import
org.springframework.boot.autoconfigure.flyway.FlywayMigrationStrategy;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.stereotype.Component;
+
+import javax.sql.DataSource;
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.List;
+
+/**
+ * Custom Flyway migration strategy that lets us perform data migration
from the original database used in the
+ * 0.1.0 release, to the new database. The data migration will be
triggered when it is determined that new database
+ * is brand new AND the legacy DB properties are specified. If the primary
database already contains the 'BUCKET' table,
+ * or if the legacy database properties are not specified, then no data
migration is performed.
+ */
+@Component
+public class CustomFlywayMigrationStrategy implements
FlywayMigrationStrategy {
--- End diff --
This is a nice solution to determining when to migrate databases. Nice work!
---
[GitHub] nifi-minifi-cpp pull request #351: MINIFICPP-523 Fixes bootstrap continue wi...
GitHub user kevdoran opened a pull request: https://github.com/apache/nifi-minifi-cpp/pull/351 MINIFICPP-523 Fixes bootstrap continue with plan prompt 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- where 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/kevdoran/nifi-minifi-cpp MINIFICPP-523-bootstrap Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi-minifi-cpp/pull/351.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 #351 commit 76b4929ee50f188459fa562ec54fa83e73997c99 Author: Kevin Doran Date: 2018-06-04T00:55:30Z MINIFICPP-523 Fixes bootstrap continue with plan prompt ---
[GitHub] nifi-registry issue #121: NIFIREG-173 Refactor metadata DB to be independent...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/121 Will review... ---
[GitHub] nifi-registry pull request #119: NIFIREG-172 Adds Swagger UI
GitHub user kevdoran opened a pull request: https://github.com/apache/nifi-registry/pull/119 NIFIREG-172 Adds Swagger UI Contains the following changes: - Adds self-hosted Swagger UI to nifi-registry-web-api WAR at /swagger/ui.html - Updates NOTICE for included ALv2 licensed source - Adds Jersey filter exclusion for resources starting with /swagger/* - Adds top-level authorizable resource type for /swagger/* - Updates ResourceAuthorizationFilter configuration to include swagger resource type - Corrects name of Position model object in Swagger specification - Corrects duplicate operationId/nickname field for methods in FlowResource and BucketFlowResource For reviewers: - Verify /nifi-registry-api/swagger/ui.html is accessible and working in unsecured mode. - Verify /nifi-registry-api/swagger/swagger.json returns a good Swagger spec. - In secured mode, an initial admin should have access, as should any user who is granted read access to the "/swagger" resource - Verify changes to NOTICE in source code and nifi-registry-assembly You can merge this pull request into a Git repository by running: $ git pull https://github.com/kevdoran/nifi-registry NIFIREG-172-swagger-ui Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi-registry/pull/119.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 #119 commit 8eef21c9481f795500a0a5d995deb2bdabf1899d Author: Kevin Doran Date: 2018-05-29T16:13:06Z NIFIREG-172 Adds Swagger UI - Adds self-hosted Swagger UI to nifi-registry-web-api WAR at /swagger/ui.html - Updates NOTICE for included ALv2 licensed source. - Adds Jersey filter exclusion for resources starting with /swagger/* - Adds top-level authorizable resource type for /swagger/* - Updates ResourceAuthorizationFilter configuration to include swagger resource type - Corrects name of Position model object in Swagger specification - Corrects duplicate operationId/nickname field for methods in FlowResource and BucketFlowResource ---
[GitHub] nifi-registry issue #112: NIFIREG-162: Support Git backed PersistenceProvide...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/112 Nice work @ijokarumawak and everyone who contributed to this. This is a major feature to have as part of NiFi Registry and will certainly be useful. ---
[GitHub] nifi issue #2685: NIFI-5163 Clearing version control info when creating a te...
Github user kevdoran commented on the issue: https://github.com/apache/nifi/pull/2685 Thanks for the quick jira & patch @bbende. Reviewing... ---
[GitHub] nifi pull request #2683: NIFI-5146 Only support HTTP or HTTPS operation for ...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2683#discussion_r186499568
--- Diff:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java
---
@@ -601,106 +601,144 @@ private void configureConnectors(final Server
server) throws ServerConfiguration
httpConfiguration.setRequestHeaderSize(headerSize);
httpConfiguration.setResponseHeaderSize(headerSize);
-if (props.getPort() != null) {
-final Integer port = props.getPort();
-if (port < 0 || (int) Math.pow(2, 16) <= port) {
-throw new ServerConfigurationException("Invalid HTTP port:
" + port);
-}
+// Check if both HTTP and HTTPS connectors are configured and fail
if both are configured
+if (bothHttpAndHttpsConnectorsConfigured(props)) {
+logger.error("NiFi only supports one mode of HTTP or HTTPS
operation, not both simultaneously. " +
+"Check the nifi.properties file and ensure that either
the HTTP hostname and port or the HTTPS hostname and port are empty");
+startUpFailure(new IllegalStateException("Only one of the HTTP
and HTTPS connectors can be configured at one time"));
+}
-logger.info("Configuring Jetty for HTTP on port: " + port);
+if (props.getSslPort() != null) {
+configureHttpsConnector(server, httpConfiguration);
+} else if (props.getPort() != null) {
+configureHttpConnector(server, httpConfiguration);
+} else {
+logger.error("Neither the HTTP nor HTTPS connector was
configured in nifi.properties");
+startUpFailure(new IllegalStateException("Must configure HTTP
or HTTPS connector"));
+}
+}
-final List serverConnectors = Lists.newArrayList();
+/**
+ * Configures an HTTPS connector and adds it to the server.
+ *
+ * @param server the Jetty server instance
+ * @param httpConfiguration the configuration object for the HTTPS
protocol settings
+ */
+private void configureHttpsConnector(Server server, HttpConfiguration
httpConfiguration) {
+String hostname = props.getProperty(NiFiProperties.WEB_HTTP_HOST);
--- End diff --
Unless I'm missing something, this should be `NiFiProperties.WEB_HTTPS_HOST`
---
[GitHub] nifi-registry pull request #112: NIFIREG-162: Support Git backed Persistence...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/112#discussion_r184700563
--- Diff: nifi-registry-docs/src/main/asciidoc/administration-guide.adoc ---
@@ -895,3 +895,167 @@ Providing 2 total locations, including
`nifi.registry.extension.dir.1`.
Example: `/etc/http-nifi-registry.keytab`
|nifi.registry.kerberos.spengo.authentication.expiration|The expiration
duration of a successful Kerberos user authentication, if used. The default
value is `12 hours`.
|
+
+== Persistence Providers
+
+NiFi Registry uses a pluggable flow persistence provider to store the
content of the flows saved to the registry. NiFi Registry provides
`<>` and `<>`.
+
+Each persistence provider has its own configuration parameters, those can
be configured in a XML file specified in <>.
+
+The XML configuration file looks like below. It has a
`flowPersistenceProvider` element in which qualified class name of a
persistence provider implementation and its configuration properties are
defined. See following sections for available configurations for each providers.
+
+.Example providers.xml
+[source,xml]
+
+
+
+
+
+persistence-provider-qualified-class-name
+property-value-1
+property-value-2
+property-value-n
+
+
+
+
+
+
+=== FileSystemFlowPersistenceProvider
+
+FileSystemFlowPersistenceProvider simply stores serialized Flow contents
into `{bucket-id}/{flow-id}/{version}` directories.
+
+Example of persisted files:
+
+Flow Storage Directory/
+âââ {bucket-id}/
+â âââ {flow-id}/
+â âââ {version}/{version}.snapshot
+âââ d1beba88-32e9-45d1-bfe9-057cc41f7ce8/
+âââ 219cf539-427f-43be-9294-0644fb07ca63/
+âââ 1/1.snapshot
+âââ 2/2.snapshot
+
+
+Qualified class name:
`org.apache.nifi.registry.provider.flow.FileSystemFlowPersistenceProvider`
+
+|
+|*Property*|*Description*
+|Flow Storage Directory|REQUIRED: File system path for a directory where
flow contents files are persisted to. If the directory does not exist when NiFi
Registry starts, it will be created. If the directory exists, it must be
readable and writable from NiFi Registry.
+|
+
+
+=== GitFlowPersistenceProvider
+
+GitFlowPersistenceProvider stores flow contents under a Git directory.
+
+In contrast to FileSystemFlowPersistenceProvider, this provider uses human
friendly Bucket and Flow names so that those files can be accessed by external
tools. However, it is NOT supported to modify stored files outside of NiFi
Registry. Persisted files are only read when NiFi Registry starts up.
+
+Buckets are represented as directories and Flow contents are stored as
files in a Bucket directory they belong to. Flow snapshot histories are managed
as Git commits, meaning only the latest version of Buckets and Flows exist in
the Git directory. Old versions are retrieved from Git commit histories.
+
+.Example persisted files
+
+Flow Storage Directory/
+âââ .git/
+âââ Bucket A/
+â âââ bucket.yml
+â âââ Flow 1.snapshot
+â âââ Flow 2.snapshot
+âââ Bucket B/
+âââ bucket.yml
+âââ Flow 4.snapshot
+
+
+Each Bucket directory contains a YAML file named `bucket.yml`. The file
manages links from NiFi Registry Bucket and Flow IDs to actual directory and
file names. When NiFi Registry starts, this provider reads through Git commit
histories and lookup these `bucket.yml` files to restore Buckets and Flows for
each snapshot version.
+
+.Example bucket.yml
+[source,yml]
+
+layoutVer: 1
+bucketId: d1beba88-32e9-45d1-bfe9-057cc41f7ce8
+flows:
+ 219cf539-427f-43be-9294-0644fb07ca63: {ver: 7, file: Flow 1.snapshot}
+ 22cccb6c-3011-4493-a996-611f8f112969: {ver: 3, file: Flow 2.snapshot}
+
+
+Qualified class name:
`org.apache.nifi.registry.provider.flow.git.GitFlowPersistenceProvider`
+
+|
+|*Property*|*Description*
+|Flow Storage Directory|REQUIRED: File system path for a directory where
flow contents files are persisted to. The directory must exist when NiFi
registry starts. Also must be initialized as a Git directory. See <> for detail.
+|Remote To Push|When a new flow snapshot is created, this persistence
provider updated files in the specified Git directory, then create a commit to
the local repository. If `Remote To Push` is defined, it also pushes to the
specified remote repository. E.g. 'origin'. To define more detailed remote spec
such as branch names, use `Refspec`. See
https://git-scm.com/
[GitHub] nifi issue #2648: NIFI-4942 Fix unit test salt assertion regex
Github user kevdoran commented on the issue: https://github.com/apache/nifi/pull/2648 Aside from the issue with the ./secure_hash.key file being created in an odd location, all other issues should be addressed now. If someone can verify (the more platforms we have coverage on, the better) that would be much appreciated. ---
[GitHub] nifi pull request #2648: NIFI-4942 Fix unit test salt assertion regex
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi/pull/2648#discussion_r182941468 --- Diff: nifi-toolkit/nifi-toolkit-encrypt-config/pom.xml --- @@ -167,10 +167,12 @@ org.apache.rat apache-rat-plugin +true src/test/resources/scrypt.py - src/test/resources/secure_hash.key - src/test/resources/secure_hash_128.key + +**/secure_hash.key +**/secure_hash_128.key --- End diff -- Ok, here's what I've got so far for the secure_hash.key file issue. It does not look trivial to find and disable the tests that could be creating this file, as I think it can come from both the existing (pre-NIFI-4942) and the recently introduced test cases. The path is hardcoded to ./secure_hash.key, but the test class does appear to try to account for that here: https://github.com/kevdoran/nifi/blob/master/nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groov... I'm not sure why that is not working on some Linux platforms. I'm wondering if it has to do with the fact that it is a static field and how the classes get loaded for these test cases, but in any case it means that any execution of the tool could create the file that we want to avoid. So options I see are: - leave the tests enabled, leave the wildcard directory for the RAT exclusions, and maybe add the file to a .gitignore listing so as to prevent it from getting accidentally committed - try to find every test that is creating this file and temporarily disable it (non-trivial from what I can tell, but maybe I am missing something obvious) - disable the entire ConfigEncryptionToolTest suite (for now) I think all of these approaches only address the immediate issues and still require a longer-term solution. ---
[GitHub] nifi pull request #2648: NIFI-4942 Fix unit test salt assertion regex
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi/pull/2648#discussion_r182934721 --- Diff: nifi-toolkit/nifi-toolkit-encrypt-config/pom.xml --- @@ -167,10 +167,12 @@ org.apache.rat apache-rat-plugin +true src/test/resources/scrypt.py - src/test/resources/secure_hash.key - src/test/resources/secure_hash_128.key + +**/secure_hash.key +**/secure_hash_128.key --- End diff -- Thanks for taking a look! I don't know enough about the tool to make a change to this behavior as it might have good reason it needs to work that way. For now I will update the PR to disable the tests, and we can take our time to discuss the best approach that looks at the tests and tool holistically. ---
[GitHub] nifi pull request #2648: NIFI-4942 Fix unit test salt assertion regex
Github user kevdoran commented on a diff in the pull request: https://github.com/apache/nifi/pull/2648#discussion_r182931924 --- Diff: nifi-toolkit/nifi-toolkit-encrypt-config/pom.xml --- @@ -167,10 +167,12 @@ org.apache.rat apache-rat-plugin +true src/test/resources/scrypt.py - src/test/resources/secure_hash.key - src/test/resources/secure_hash_128.key + +**/secure_hash.key +**/secure_hash_128.key --- End diff -- Thanks @ijokarumawak. That confirms what I and others have been able to gather. It looks like that file is created by the command line tool that is under test. I don't think there is a way to change its location, other than perhaps changing the working directory of the test prior to executing the tool. Reliably removing the file would be a good approach as well. We may also want to disable these tests to get master building reliably on all platforms and then re-enable them once we have sorted this out. Thanks for jumping in to assist! ---
[GitHub] nifi issue #2648: NIFI-4942 Fix unit test salt assertion regex
Github user kevdoran commented on the issue: https://github.com/apache/nifi/pull/2648 Ok, thanks. good to know. I'll make that change and also fix or disable those tests. Will update soon ---
[GitHub] nifi issue #2648: NIFI-4942 Fix unit test salt assertion regex
Github user kevdoran commented on the issue: https://github.com/apache/nifi/pull/2648 Thanks for taking a look @joewitt. I'll try a build on Linux and try to reproduce that. I agree on placing the consoleOutput=true setting in the root pom. Does it also need to be included in every submodule that respecifies this plugin (i.e., the modules that configure their own exclusion)? ---
[GitHub] nifi pull request #2648: NIFI-4942 Fix unit test salt assertion regex
GitHub user kevdoran opened a pull request: https://github.com/apache/nifi/pull/2648 NIFI-4942 Fix unit test salt assertion regex Thank you for submitting a contribution to Apache NiFi. 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 NIFI- where 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: - [ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder? - [ ] Have you written or updated unit tests to verify your 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, including the main LICENSE file under nifi-assembly? - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly? - [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties? ### 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/kevdoran/nifi NIFI-4942-fix-test Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/2648.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 #2648 commit a906c7e7d10b3b478aee165077f87507adeb94c0 Author: Kevin Doran <kdoran@...> Date: 2018-04-19T17:54:23Z NIFI-4942 Fix unit test salt assertion regex ---
[GitHub] nifi-registry issue #89: NIFIREG-120 Basic Docker Image Support
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/89 FYI to those following this PR. Thanks to @apiri an image built using these scripts has been published to DockerHub under the Apache group: https://hub.docker.com/r/apache/nifi-registry/tags/ ---
[GitHub] nifi-registry pull request #108: NIFIREG-158 Added ability to retrieve flow ...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-registry/pull/108#discussion_r180112539
--- Diff:
nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/FlowResource.java
---
@@ -62,4 +85,214 @@ public Response getAvailableFlowFields() {
return Response.status(Response.Status.OK).entity(fields).build();
}
+@GET
+@Path("{flowId}")
+@Consumes(MediaType.WILDCARD)
+@Produces(MediaType.APPLICATION_JSON)
+@ApiOperation(
+value = "Gets a flow",
+response = VersionedFlow.class,
+extensions = {
+@Extension(name = "access-policy", properties = {
+@ExtensionProperty(name = "action", value =
"read"),
+@ExtensionProperty(name = "resource", value =
"/buckets/{bucketId}") })
+}
+)
+@ApiResponses({
+@ApiResponse(code = 400, message =
HttpStatusMessages.MESSAGE_400),
+@ApiResponse(code = 401, message =
HttpStatusMessages.MESSAGE_401),
+@ApiResponse(code = 403, message =
HttpStatusMessages.MESSAGE_403),
+@ApiResponse(code = 404, message =
HttpStatusMessages.MESSAGE_404),
+@ApiResponse(code = 409, message =
HttpStatusMessages.MESSAGE_409) })
+public Response getFlow(
+@PathParam("flowId")
+@ApiParam("The flow identifier")
+final String flowId) {
+
+final VersionedFlow flow = registryService.getFlow(flowId);
+
+// this should never happen, but if somehow the back-end didn't
populate the bucket id let's make sure the flow isn't returned
+if (StringUtils.isBlank(flow.getBucketIdentifier())) {
+throw new IllegalStateException("Unable to authorize access
because bucket identifier is null or blank");
+}
+
+authorizeBucketAccess(RequestAction.READ,
flow.getBucketIdentifier());
--- End diff --
When I try this endpoint with an unauthorized user, I get the following
response back:
```
HTTP/1.1 403 Forbidden
Connection: close
Date: Mon, 09 Apr 2018 14:08:27 GMT
Content-Type: text/plain
Content-Length: 101
Server: Jetty(9.4.3.v20170317)
Unable to view Bucket with ID 6eaeae9c-dbdb-4af3-a98e-4f3b880a0fb2. Contact
the system administrator.
```
The 403 status code is good, but I'm not sure about the error message in
the response. If someone is attempting to access the flow through /flows/{id},
I don't think the server should return the bucket id containing the flow, as
that's leaking information the user would not otherwise have access to. It's a
fairly harmless piece of information on it's own, but in a multi-tenant
scenario it could reveal more than the owner of the bucket would like,
especially if correlated with other information an attacker is able to obtain.
It's probably not a huge issue, but if you agree, we could strip this by
wrapping the call to authorizeBucketAccess() in a try catch that obscures the
error message returned in the response body. We would have to do this for all
the GET methods in the FlowResource. Something like:
```
try {
authorizeBucketAccess(RequestAction.READ, flow.getBucketIdentifier());
} catch (AccessDeniedException e) {
throw new AccessDeniedException("User not authorized to view the
specified flow");
}
By adding the root throwable to the cause of a custom exception, we could
even keep the root cause with the bucket id in the logs for easier admin
troubleshooting.
Now that I think about it, that approach for customizing HTTP response
error messages to differ from internal/logged error message probably be a
better approach than the solution that we came up with in PR #99, which
inadvertently introduced a "log & throw" pattern in order to maintain resource
ids in the logs.
---
[GitHub] nifi-registry issue #108: NIFIREG-158 Added ability to retrieve flow directl...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/108 @bbende alright will wait until it's ready. thanks for letting me know! ---
[GitHub] nifi-registry issue #108: NIFIREG-158 Added ability to retrieve flow directl...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/108 Will review... ---
[GitHub] nifi-minifi pull request #120: MINIFI-448 C2 server functionality and intern...
Github user kevdoran closed the pull request at: https://github.com/apache/nifi-minifi/pull/120 ---
[GitHub] nifi-minifi pull request #120: MINIFI-448 C2 server functionality and intern...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/120#discussion_r177852495
--- Diff:
minifi-c2/minifi-c2-framework/src/test/groovy/org/apache/nifi/minifi/c2/core/service/StandardC2ProtocolServiceSpec.groovy
---
@@ -0,0 +1,164 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.core.service
+
+import
org.apache.nifi.minifi.c2.api.provider.heartbeat.HeartbeatPersistenceProvider
+import org.apache.nifi.minifi.c2.model.*
+import spock.lang.Specification
+
+class StandardC2ProtocolServiceSpec extends Specification {
--- End diff --
Good question, @jzonthemtn. There is no technical/functional reason these
tests couldn't be written as Java junit tests. The groovy framework I'm using
here, [Spock](https://github.com/spockframework/spock), offers a few advantages:
- Detailed error messages for failed tests/assertions
- A reasonable terse/structured syntax that allows tests to serve as
feature/method requirements/specifications. This was really beneficial in
writing
[StandardC2ServiceSpec](https://github.com/apache/nifi-minifi/pull/120/files#diff-f485f870b41384487a26ff065fd25467),
which is extremely repetitive. I wanted the tests to be short and
self-descriptive so I could keep track of what combinations were covered.
- Powerful mocking capabilities (with some advantages over Java libs such
as Mockito)
- built-in test templating and data driven parameterization (although I did
not use that feature in this test cases).
---
[GitHub] nifi-minifi pull request #120: MINIFI-448 C2 server functionality and intern...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/120#discussion_r177827045
--- Diff:
minifi-c2/minifi-c2-framework/src/main/java/org/apache/nifi/minifi/c2/core/service/StandardC2Service.java
---
@@ -0,0 +1,494 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.core.service;
+
+import
org.apache.nifi.minifi.c2.api.provider.agent.AgentPersistenceProvider;
+import
org.apache.nifi.minifi.c2.api.provider.device.DevicePersistenceProvider;
+import
org.apache.nifi.minifi.c2.api.provider.operations.OperationPersistenceProvider;
+import org.apache.nifi.minifi.c2.core.exception.ResourceNotFoundException;
+import org.apache.nifi.minifi.c2.model.Agent;
+import org.apache.nifi.minifi.c2.model.AgentClass;
+import org.apache.nifi.minifi.c2.model.AgentManifest;
+import org.apache.nifi.minifi.c2.model.Device;
+import org.apache.nifi.minifi.c2.model.OperationRequest;
+import org.apache.nifi.minifi.c2.model.OperationState;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.ConstraintViolationException;
+import javax.validation.Validator;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+@Service
+public class StandardC2Service implements C2Service {
+
+private static final Logger logger =
LoggerFactory.getLogger(StandardC2Service.class);
+
+private final AgentPersistenceProvider agentPersistenceProvider;
+private final DevicePersistenceProvider devicePersistenceProvider;
+private final OperationPersistenceProvider
operationPersistenceProvider;
+private final Validator validator;
+
+private final ReentrantReadWriteLock lock = new
ReentrantReadWriteLock();
--- End diff --
Great points, @bbende. Thanks for the input.
This is probably too limiting as you point. One of the things I like about
trying to handle concurrency in the service layer, as we did for registry, is
that there is less burden placed on the implementations of the persistence
providers that are called from the service methods (though if they are DB
backed most DB engines will take care of that for you). But I agree, as
implemented here, it's too much of a performance trade off if we want to
support high concurrency / number of clients for a C2 server instance.
I'll take another pass at this and look at it on more of a case-by-case
basis.
---
[GitHub] nifi-minifi pull request #120: MINIFI-448 C2 server functionality and intern...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/120#discussion_r177824172
--- Diff:
minifi-c2/minifi-c2-framework/src/main/java/org/apache/nifi/minifi/c2/api/provider/device/DevicePersistenceProvider.java
---
@@ -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.
+ */
+package org.apache.nifi.minifi.c2.api.provider.device;
+
+import org.apache.nifi.minifi.c2.api.provider.Provider;
+import org.apache.nifi.minifi.c2.model.Device;
+
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * NOTE: Although this interface is intended to be an extension point, it
is not yet considered stable and thus may
+ * change across releases until the the C2 Server APIs mature.
+ *
+ * TODO, we may want to consider creating a separate entity model rather
than reusing the REST API object model.
+ * Currently, this design assumes the Provider implementation will do that
translation.
+ * This requires adding a dependency on minifi-c2-commons here for the
data model.
+ */
+public interface DevicePersistenceProvider extends Provider {
--- End diff --
> Would it make sense to have a base interface kind of like the Spring Data
repositories
I think this makes a lot of sense. I'm going to create this base interface
type as you suggested. Are there any other methods you can think of that should
be included the base interface? (It has everything needed for the current
framework invocations, but expecting third-parties might provide an
implementation, is there anything else we can anticipate wanting provided?)
> So the only pluggable provider is the top-level AgentPersistenceProvider,
but then it is up to that provider to provider implementations of the
sub-providers.
Yeah I like this approach. Going to do that for now and we can refine it
(if needed) as these provider interfaces mature a bit.
---
[GitHub] nifi-registry issue #107: NIFIREG-157 Adding boolean to VersionedPropertyDes...
Github user kevdoran commented on the issue: https://github.com/apache/nifi-registry/pull/107 Looks good, will merge to master. Thanks for adding this @bbende ---
[GitHub] nifi-minifi pull request #120: MINIFI-448 C2 server functionality and intern...
GitHub user kevdoran opened a pull request: https://github.com/apache/nifi-minifi/pull/120 MINIFI-448 C2 server functionality and internal interfaces The C2 service layer is the part of the C2 framework that backs the REST API to provide the business logic. This commit also defines a few preliminary provider interfaces for persistence based on what is required by the service layer. A reference, in-memory implementation of the provider interfaces is included. --- Thank you for submitting a contribution to Apache NiFi - MiNiFi. 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- where 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: - [ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi-minifi folder? - [ ] Have you written or updated unit tests to verify your 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, including the main LICENSE file under minifi-assembly? - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under minifi-assembly? ### 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/kevdoran/nifi-minifi MINIFI-448-service-layer Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi-minifi/pull/120.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 #120 ---
[GitHub] nifi-minifi pull request #119: MINIFI-447 - Adding FlowMapper and FlowRetrie...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/119#discussion_r176803697
--- Diff:
minifi-c2/minifi-c2-framework/src/main/java/org/apache/nifi/minifi/c2/core/service/flow/client/NiFiRegistryClientFactory.java
---
@@ -0,0 +1,126 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.core.service.flow.client;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Validate;
+import org.apache.nifi.minifi.c2.properties.C2Properties;
+import org.apache.nifi.registry.client.NiFiRegistryClient;
+import org.apache.nifi.registry.client.NiFiRegistryClientConfig;
+import org.apache.nifi.registry.client.impl.JerseyNiFiRegistryClient;
+import org.apache.nifi.registry.security.util.KeystoreType;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+/**
+ * This class does not follow the typical "factory bean" pattern of having
a method annotated with @Bean because
+ * we want to support running a C2 server that may not be configured to do
flow deployments, which means we don't want
+ * to create a NiFiRegistryClient during start-up because the registry URL
may not be populated.
+ *
+ * An instance of this class can be injected into other services that need
a NiFiRegistryClient, and if those services
+ * get called then they can use this class to lazily obtain an instance,
which can then throw an exception if the
+ * appropriate configuration is not provided.
+ */
+@Component
+public class NiFiRegistryClientFactory {
+
+private volatile NiFiRegistryClient client;
+
+private final C2Properties c2Properties;
+
+@Autowired
+public NiFiRegistryClientFactory(final C2Properties c2Properties) {
+this.c2Properties = c2Properties;
+Validate.notNull(this.c2Properties);
--- End diff --
This is minor, but I think \@ Autowired beans are validated to be not null
by default, unless annotated with \@ Nullable, right?
---
[GitHub] nifi-minifi pull request #119: MINIFI-447 - Adding FlowMapper and FlowRetrie...
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/119#discussion_r176815905
--- Diff:
minifi-c2/minifi-c2-framework/src/main/java/org/apache/nifi/minifi/c2/core/service/flow/mapping/FlowMapper.java
---
@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.core.service.flow.mapping;
+
+import org.apache.nifi.minifi.c2.model.FlowUri;
+
+import java.util.Optional;
+
+/**
+ * Provides mappings from an agent class to the URI of a versioned flow.
+ */
+public interface FlowMapper {
+
+/**
+ * Gets the flow mapping information for the given agent class.
+ *
+ * @param agentClassName the name of an agent class
+ * @return the flow mapping information for the given agent class
+ * @throws FlowMapperException if an error occurs getting the mapping
+ */
+Optional getFlowMapping(String agentClassName) throws
FlowMapperException;
--- End diff --
Nice, I like where you landed on this interface; that will work nicely.
---
[GitHub] nifi-minifi pull request #118: MINIFI-444 C2 Data Model and REST API
Github user kevdoran closed the pull request at: https://github.com/apache/nifi-minifi/pull/118 ---
[GitHub] nifi-minifi pull request #118: MINIFI-444 C2 Data Model and REST API
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/118#discussion_r176297389
--- Diff:
minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/extension/ExtensionComponent.java
---
@@ -0,0 +1,85 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.model.extension;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+
+import java.util.List;
+import java.util.Set;
+
+/**
+ * A component provided by an extension bundle
+ */
+@ApiModel
+public class ExtensionComponent extends DefinedType {
+
+// TODO, does arch/binary/compiler metadata need to be added here?
--- End diff --
Ok, so here is what I came up with for now:
An extension component is uniquely identified by (bundle, type), where
bundle = (group, artifact, version) and type is a fully qualified class name.
Bundle is optional in the case that you have an extension component outside of
a bundle (common in the C++ case for now), in which case the fully qualified
type must be globally unique.
An extension component implementation is uniquely identified (bundle, type,
build) where build = (version, revision, timestamp, target architecture,
compiler, compiler flags)
In general, a flow designer should only have to target an extension
component in order to author a flow. Whether or not a flow can run on a given
agent depends on if the extension component has an available implementation on
that agent.
It's still a bit hand-wavy (at least for now), but I think this distinction
should work once we have more dynamic authoring and deployment logic that
leverages a fully featured device registry and extension registry. For now, we
will focus on agent class name compatibility, so we will have to assume that
manifests for classes don't change (or at least, don't remove capabilities) and
use the intersection of all agent manifest capabilities when designing against
a an agent class label.
I need to flesh it out a bit more, but I think it should work. To get a
sense of what some of this looks like in code, look at the equals/hashcode
methods for DefinedType and ExtensionComponent after I push my next commit.
---
[GitHub] nifi-minifi pull request #118: MINIFI-444 C2 Data Model and REST API
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/118#discussion_r176286360
--- Diff:
minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/FlowUri.java
---
@@ -0,0 +1,54 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.model;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+
+@ApiModel("Uniform Resource Identifier for NiFi Versioned Flows saved to a
NiFi Registry")
+public class FlowUri {
--- End diff --
Will do!
---
[GitHub] nifi-minifi pull request #118: MINIFI-444 C2 Data Model and REST API
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/118#discussion_r176281479
--- Diff:
minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/FlowUri.java
---
@@ -0,0 +1,54 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.model;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+
+@ApiModel("Uniform Resource Identifier for NiFi Versioned Flows saved to a
NiFi Registry")
+public class FlowUri {
--- End diff --
Yes it is {base_url}/buckets/{bucketId}/flows/{flowId}. We could use that
in place of this object if preferred. or we could use this object but have a
constructor / toString that is capable of going from/to that form.
---
[GitHub] nifi-minifi pull request #118: MINIFI-444 C2 Data Model and REST API
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/118#discussion_r176280748
--- Diff:
minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/FlowStatus.java
---
@@ -0,0 +1,51 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.model;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+
+import java.util.Map;
+
+/**
+ * Status of the aspects of the flow that are controllable by the C2
server, ie:
+ * - Processors that can be started/stopped and their current state
+ * - Queues that can be cleared and their current state
+ */
+@ApiModel
+public class FlowStatus {
--- End diff --
OK, I got it. The current structure assumes to much and is limiting. Will
take another look at that, thanks.
Yes, I don't think these DTOs need to worry about concurrent access.
Official model and state and would be managed at a lower level in the C2 server
(persisted or cached and accessed through thread safe interfaces). These
objects represent a snapshot copy for I/O. It should become clearer in my next
PR that adds some of the service and persistence layer foundations.
---
[GitHub] nifi-minifi pull request #118: MINIFI-444 C2 Data Model and REST API
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/118#discussion_r176279968
--- Diff:
minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/extension/BundleManifest.java
---
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.model.extension;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+
+import java.util.List;
+
+@ApiModel
+public class BundleManifest {
+
--- End diff --
OK, I like that too. I'll add top level components to the AgentManifest,
and devs can choose which makes more sense for them. Flow Designer can adapt to
both.
---
[GitHub] nifi-minifi pull request #118: MINIFI-444 C2 Data Model and REST API
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/118#discussion_r176278810
--- Diff:
minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/C2Operation.java
---
@@ -0,0 +1,99 @@
+/*
+ * 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.
+ */
+
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.model;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+
+import javax.validation.constraints.NotBlank;
+import java.util.Map;
+
+@ApiModel
+public class C2Operation {
+
+private String identifier;
--- End diff --
OK got it, thanks. It seems that a protocol distinction here is that
operations in the top level requestedOperations list are always executed, but
nested operations only execute if their parent succeeds?
---
[GitHub] nifi-minifi pull request #118: MINIFI-444 C2 Data Model and REST API
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/118#discussion_r176278082
--- Diff:
minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/extension/ExtensionComponent.java
---
@@ -0,0 +1,85 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.model.extension;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+
+import java.util.List;
+import java.util.Set;
+
+/**
+ * A component provided by an extension bundle
+ */
+@ApiModel
+public class ExtensionComponent extends DefinedType {
+
+// TODO, does arch/binary/compiler metadata need to be added here?
--- End diff --
> ... would it make this an implementation of N types of components based
on flags/architectures or a singular object which is encapsulated elsewhere,
with the necessary information...
Great question, and one I am still grappling with myself. I'd like to
abstract these details away from the user as well and make it as easy as
possible for both extension authors and extension users. I think I would lean
towards something along the lines that an extension should define an interface
(eg, the metadata in this model) that can be implemented on multiple platforms.
In otherwords, if I choose to support the JVM, x86, and ARM, I could publish
that as a single extension (MyCompany, MyExtension, v1.0) and there is some
metadata that lets the MiNiFi/NiFi ecosystem know about the platform-specific
binaries. I think this is heading down the path of a full-fledged Extension
Registry though, which might be biting of a bit to much for this initial
version of the C2 effort (we aren't even attempting to host binaries or
facilitate centralized distribution and dynamic installation). That said, If
anyone can think of a minimal version of that concept that gets us on the right
path,
I'll be happy to include it!
---
[GitHub] nifi-minifi pull request #118: MINIFI-444 C2 Data Model and REST API
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/118#discussion_r175938847
--- Diff:
minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/AgentRepositoryStatus.java
---
@@ -0,0 +1,63 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.model;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+
+@ApiModel
+public class AgentRepositoryStatus {
+
+private Long size;
+private Long sizeMax;
+private Long count;
+private Long countMax;
--- End diff --
For now I went with these field names:
private Long size;
private Long sizeMax;
private Long dataSize;
private Long dataSizeMax;
Thoughts?
---
[GitHub] nifi-minifi pull request #118: MINIFI-444 C2 Data Model and REST API
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/118#discussion_r175844957
--- Diff:
minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/extension/BundleManifest.java
---
@@ -0,0 +1,65 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.model.extension;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+
+import java.util.List;
+
+@ApiModel
+public class BundleManifest {
+
--- End diff --
::sigh:: I typed a long response that I think got lost before I hit the
button.
The gist of it is -> I need to think about this. Currently the data model
requires defining an extension in the context of a bundle. If there is no
actual bundle, you could use a default bundle (I even included a constructor
`defaultBundle()` to create one) that indicates there is no actual bundle used.
Alternatively, we could add ExtensionComponents directly to AgentManifest
so you could include them without creating fake bundles to act as containers.
Thoughts?
---
[GitHub] nifi-minifi pull request #118: MINIFI-444 C2 Data Model and REST API
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi/pull/118#discussion_r175843712
--- Diff:
minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/extension/DefinedType.java
---
@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.minifi.c2.model.extension;
+
+import io.swagger.annotations.ApiModel;
+import io.swagger.annotations.ApiModelProperty;
+
+/**
+ * A reference to a defined type identified by bundle and fully qualified
class type identifiers
+ */
+@ApiModel
+public class DefinedType {
+
+private String group;
+private String artifact;
+private String version;
+private String type;
--- End diff --
I think in practice it will just be a class that is deployable, i.e., via a
flow config.yaml. So basically a processor, controller service, or reporting
task.
But it could be more than that. For example, in NiFi, some of the core
framework interface implementations are actually built as NARs and deployed by
including them in the /lib dir. So givent that the model in the
`org.apache.nifi.minifi.c2.model.extension` package is designed to be a
prototype for a model that could eventually live in an Extension Registry,
these could be used in the NiFi case as well. But for our purposes, just the
types that would be used as Flow Components
---
