MINIFI-444 C2 Data Model and REST API #118

kevdoran · 2018-03-16T19:58:35Z

Adds the foundation of a data model and REST API that would support
discussed features such as a MiNiFi Agent C2 Protocol and
MiNiFi Flow Designer.

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-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically master)?
Is your initial contribution a single, squashed commit?

For code changes:

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?
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.

The foundation of a data model and REST API that would support discussed features such as a MiNiFi Agent C2 Protocol and MiNiFi Flow Designer.

phrocker

Thanks for putting this up. Took a cursory review. Will take a look later/this weekend if I have a chance.

phrocker · 2018-03-16T20:01:10Z

minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/AgentInfo.java

+
+    private String identifier;
+    // TODO, do we also need identity. e.g., cert DN
+    private String agentClass;


Any reason why this is not more strongly typed?

[Some background to those who are new to the terminology that's starting to formulate around these features: In this context, class != OO type. Rather, it's intended as a category/purpose label for a MiNiFi agent that could be used for a variety of purposes from a C2 server: e.g., seeing summary views of a class, showing all agents of a particular class, scheduling a bulk operation (such as deploying flow update) to all agents that match a particular class.]

In the data model, I do have an AgentClass type that could be used here, but I think that will really just be metadata that one can associate with a class in the C2 server. We'll have to enforce agent class name uniqueness, so that in contexts where the full agent class metadata is not needed, just the class name can be used.

phrocker · 2018-03-16T20:01:20Z

minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/AgentInfo.java

+    private String identifier;
+    // TODO, do we also need identity. e.g., cert DN
+    private String agentClass;
+    private Collection<String> tags;


What are tags?

I was thinking it would be nice for an operator to be able to tag agents with arbitrary labels that they can then use as filters in future queries. I'm not sure those tags would be just on the C2 server in the agents it is tracking, or if they could also be set on agents when they are provisioned and deployed, similar to the agent class labels.

Thanks for calling this out. Now that I think about it, I'd like to drop this field for now so that we can properly discuss (on the mailing list, JIRA, etc) if this is valuable and if so how to implement it.

Remove tags from AgentInfo

phrocker · 2018-03-16T20:02:10Z

minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/AgentManifest.java

+@ApiModel
+public class AgentManifest {
+
+    private String identifier;


is this referential to the identifiers above?

No, this is a new identifier that will only be used by the C2 server. It assumes the agent manifests are going to be persistable and reusable. It probably would not be present when an agent manifest is being uploaded to a C2 server (e.g., in a C2 heartbeat from a MiNiFi agent or as the output of a MiNiFi agent build that some build script uploads), but when the C2 server persists it, it will serve as a unique ID that can be used to retrieve that agent manifest again.

The primary use case would be when using a Flow Designer in NiFi, the user chooses an agent class name from a list. That selection then returns one or more agent manifests that are associated with that class name. So the way I am picturing it the mapping of agent to agent manifest depends on context: from the perspecitive of an agent instance, it's going to be 1-to-1 where the manifest is always what it currently consists of. from the c2 server's perspective, it will be 1-to-many, where it is keeping track of all manifests for a given class name based on what has been uploaded to it.

These are still very new concepts, so if you have an input to help formulate them that would be welcome! This is just an initial idea and I tried to represent it the way that will support this use case. I'm sure it will change a bit before the minifi-c2-server branch could be considered "done" and merged to master.

phrocker · 2018-03-16T20:07:52Z

...2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/AgentRepositoryStatus.java

+@ApiModel
+public class AgentRepositoryStatus {
+
+    private Long size;


keep in mind that java is a 64-bit two's complement so you could get values here that exceed the max. Should either comment re this or use Long's unsigned comparator methods.

Excellent point. Will review this and update

For numeric primitive types, should document how it will be interpreted so that serialized values are valid and interpreted correctly when deserialized by the Java C2 (e.g., signed vs unsigned)

phrocker · 2018-03-16T20:09:43Z

...2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/AgentRepositoryStatus.java

+    private Long size;
+    private Long sizeMax;
+    private Long count;
+    private Long countMax;


Count max seems confusing. I imagine it's not the max count seen. Realize you set a description in the model prop annotation, but you probably should tend toward a different name. Similarly sizemax and countmax seem to have overlap. Would storageMax and size max better reflect their intent?

I'm not tied to the names of these fields. I agree that storageMax is better than sizeMax and that 'size' in general is ambiguous without reading the description. Better to avoid confusion as you point out.

I want to make this change so that the names stay consistent with the FlowQueueStatus fields if possible. My understanding (correct me if this is wrong) is that both queues and repositories have two important metrics:

number of items they currently contain

the data/storage footprint of those items

and that, for both queues and repositories, both of these can be given configurable caps (i.e., the "max value").

If that understanding is not correct, let me know as I'll need to correct these model classes. If it is correct, what would be the best names to use that would be consistent with how these values are labeled in MiNiFi's configuration?

For now I went with these field names:

private Long size; private Long sizeMax; private Long dataSize; private Long dataSizeMax;

Thoughts?

phrocker · 2018-03-16T20:33:06Z

...i-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/extension/ExtensionComponent.java

+    }
+
+    public void setProvidedApiImplementations(List<DefinedType> providedApiImplementations) {
+        this.providedApiImplementations = providedApiImplementations;


How is this "API" implementation used? What if api implementations change will we need to require that an agreed upon versioning is used ? Is this a public API?

The intention of this field is to allow a component to declare that it provides an implementation of an interface defined by another DefinedType.

The goal (the details of which can change) is to allow bundles to declare interfaces/headers as DefinedTypes that other components can provide implementations for.

This is used in the NiFi UI, so will also likely be useful in the Flow Designer. In NiFi, it is used so that property descriptors can mark a property as requiring an API implementation selection as its value, so that those options can be presented in a drop down selection. That is one thought of how this will be used in this context.

Another possible use case of this is defining an interface in version 1.0 of and extension bundle and letting implementations of that interface explicitly declare support for in future versions of the bundle. For example, you could have:

MyBundle v1.0.0 apis: [org::apache:nifi:minifi::DatabaseService] controllerServices: org::apache:nifi:minifi::DatabaseServiceImpl provides: [MyBundle/v1.0.0/org::apache:nifi:minifi::DatabaseService] MyBundle v1.0.1 controllerServices: org::apache:nifi:minifi::DatabaseServiceImpl provides: [MyBundle/v1.0.0/org::apache:nifi:minifi::DatabaseService] MyBundle v1.1.0 apis: [org::apache:nifi:minifi::DatabaseService] controllerServices: org::apache:nifi:minifi::DatabaseServiceImpl provides: [MyBundle/v1.0.0/org::apache:nifi:minifi::DatabaseService, MyBundle/v1.1.0/org::apache:nifi:minifi::DatabaseService] MyBundle v2.0.0 apis: [org::apache:nifi:minifi::DatabaseService] controllerServices: org::apache:nifi:minifi::DatabaseServiceImpl provides: [MyBundle/v2.0.0/org::apache:nifi:minifi::DatabaseService]

In this example, an API is defined in a 1.0, improved in a patch release, evolved in a minor release, and then ultimately rethought with breaking changes in a 2.0. The implementation of the ControllerService API, which happens to be in the same bundle in this example, can declare compatibility with current or previous versions.

phrocker · 2018-03-16T20:34:31Z

...i-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/extension/PropertyDescriptor.java

+    private Boolean required;
+    private Boolean sensitive;
+    private Boolean supportsEl;
+    private DefinedType identifiesControllerServiceApi;


Should this be a boolean?

No, but I can see how the field name is misleading. The intention of this field is to indicate if the property value identifies a Controller Service API, and if so, which DefinedType that API is. This is used in NiFi to present a drop down menu of any ExtensionComponents that implement that API.

See the comment about which is related to this field.

If this API definition+implementation metadata complexity is not needed in this initial version of this workflow, or if it is too difficult to provide, we could leave it out and reintroduce it in a future version of C2 server.

phrocker · 2018-03-16T20:36:04Z

...c2/minifi-c2-web-api/src/main/java/org/apache/nifi/minifi/c2/web/api/C2ProtocolResource.java

+import javax.ws.rs.core.Response;
+
+@Component
+@Path("/c2-protocol")


Is there going to be a transformation layer for different protocol types? Will that imply live in the protocol implementation?

The internal architecture of the C2 Server will be based on these layered:

Web/Network Protocols (REST, MQTT, etc) Services Providers (e.g., database or persistence providers)

The bulk of the C2 Protocol implementation logic will live in the service layer, which will expose Java APIs for the Web protocol layers to interact will. The Web protocol implementations will provide very minimal/lightweight adapters, such as deserializing whatever wire-format they use into the in-memory objects that are required for calling the service layer API. External clients, such as MiNiFi agents, will only interact with the web protocols we choose to define, expose, and support. Everything else at the service level and below is internal to the C2 server implementation.

phrocker · 2018-03-16T20:37:22Z

minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/C2OperationAck.java

+public class C2OperationAck {
+
+    private String operationId;
+    // TODO, result, outcome, or status of the operation? eg SUCCESS/FAIL?


yeah agreed. probably need something to provide more information.

phrocker · 2018-03-16T20:38:28Z

...2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/extension/DefinedType.java

+    @ApiModelProperty(
+            value = "The fully-qualified class type",
+            required = true,
+            notes = "For example, 'org.apache.nifi.GetFile' or 'org::apache:nifi::minifi::GetFile'")


How will this be used to define an API?

See above, it's really only intended to declare that a bundle contains an API definition, not the API definition itself. Presumable the interface type would reference a fully qualified class interface defined in a .h or .java file.

kevdoran · 2018-03-19T20:27:55Z

...c2/minifi-c2-web-api/src/main/java/org/apache/nifi/minifi/c2/web/api/AgentClassResource.java

+            response = AgentClass.class,
+            responseContainer = "List"
+    )
+    public Response getAgentClasses() {


Add ReadOnly NiFi Registry URL, Bucket, and Flow ID metadata when retrieving Agent classes (so that the flow designer knows where to read/save a flow)

kevdoran · 2018-03-20T22:12:42Z

@phrocker Thanks for all the review feedback! You caught a lot of improvements, which I really appreciate. I've pushed an update to my branch that addresses a lot of it and I left a few questions. in reply to your comments.

If you have a chance to re-review that would be great. If you see any major issues that you would like me to address before merging, let me know and I'm happy to address them. Otherwise, I'd like to get this merged to the minifi-c2-server branch as a starting point, and we can tackle improvements (or unanswered questions for the parts that are still formulating) in follow up PRs prior to merging minifi-c2-server to master.

Thanks!

apiri

Overall looks like a great foundation. I had some minimal comments and I also saw there were a few items that @kdevdoran was looking for some additional insights on @phrocker.

apiri · 2018-03-21T23:06:43Z

...inifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/extension/BundleManifest.java

+
+@ApiModel
+public class BundleManifest {
+


I think I like the idea of the direct access to add components. Bundles are nice for the Java side of the house but don't necessarily map well to C++. From the standpoint of the flow designer, the notions of bundles should be minimal but some defined tuple (class, version, etc) would be all that matters as a keyed extension (which has a fuller model as outlined with the other work that has gone on in this PR).

apiri · 2018-03-21T23:10:51Z

minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/FlowUri.java

+import io.swagger.annotations.ApiModelProperty;
+
+@ApiModel("Uniform Resource Identifier for NiFi Versioned Flows saved to a NiFi Registry")
+public class FlowUri {


Not sure of how this is represented by Registry but is there an actual URI form?

https://registry.myorganization.org/nifi-registry//?

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.

I dig the constructor/toString idea. Gives us more flexibility and can tack things on as needed for whatever reason.

apiri · 2018-03-21T23:13:04Z

minifi-c2/minifi-c2-commons/src/main/java/org/apache/nifi/minifi/c2/model/Device.java

+public class Device extends DeviceInfo {
+
+    private String name;
+    private String firstSeen;  // TODO, decide on standard JSON format for timestamps in the C2 REST API


My inclination would be to just do this in epoch millis UTC and munge it however needed.

apiri · 2018-03-22T15:38:18Z

reviewing latest commit

apiri · 2018-03-22T16:16:16Z

Changes look good here. Will get this merged. Thanks!

The foundation of a data model and REST API that would support discussed features such as a MiNiFi Agent C2 Protocol and MiNiFi Flow Designer. This closes apache#118. Signed-off-by: Aldrin Piri <aldrin@apache.org>

apiri · 2018-03-22T16:21:02Z

@kevdoran This was merged in 631b599. As it is not in master this won't auto close, so if you could close this PR out at your convenience it would be appreciated!

kevdoran · 2018-03-22T16:22:00Z

Closed. Thanks @apiri and @phrocker for helping me get this in good shape.

MINIFI-444 C2 Data Model and REST API

b765407

The foundation of a data model and REST API that would support discussed features such as a MiNiFi Agent C2 Protocol and MiNiFi Flow Designer.

phrocker reviewed Mar 16, 2018

View reviewed changes

kevdoran commented Mar 19, 2018

View reviewed changes

MINIFI-444 Changes based on review feedback

d26f4e7

kevdoran force-pushed the MINIFI-444-c2-data-model branch from bc8d548 to d26f4e7 Compare March 20, 2018 22:18

MINIFI-444 Adds missing Swagger annotations to Device

ae6483d

apiri reviewed Mar 21, 2018

View reviewed changes

MINIFI-444 Additional changes based on review feedback

f555130

kevdoran closed this Mar 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MINIFI-444 C2 Data Model and REST API #118

MINIFI-444 C2 Data Model and REST API #118

kevdoran commented Mar 16, 2018 •

edited

phrocker left a comment

phrocker Mar 16, 2018

kevdoran Mar 19, 2018

phrocker Mar 16, 2018

kevdoran Mar 19, 2018 •

edited

phrocker Mar 16, 2018

kevdoran Mar 19, 2018 •

edited

phrocker Mar 16, 2018

kevdoran Mar 19, 2018 •

edited

phrocker Mar 16, 2018

kevdoran Mar 19, 2018 •

edited

kevdoran Mar 20, 2018

phrocker Mar 16, 2018

kevdoran Mar 19, 2018

phrocker Mar 16, 2018

kevdoran Mar 19, 2018

phrocker Mar 16, 2018

kevdoran Mar 19, 2018

phrocker Mar 16, 2018

phrocker Mar 16, 2018

kevdoran Mar 19, 2018

kevdoran Mar 19, 2018 •

edited

kevdoran commented Mar 20, 2018

apiri left a comment

apiri Mar 21, 2018

apiri Mar 21, 2018

kevdoran Mar 22, 2018

apiri Mar 22, 2018

kevdoran Mar 22, 2018

apiri Mar 21, 2018

apiri commented Mar 22, 2018

apiri commented Mar 22, 2018

apiri commented Mar 22, 2018

kevdoran commented Mar 22, 2018

MINIFI-444 C2 Data Model and REST API #118

MINIFI-444 C2 Data Model and REST API #118

Conversation

kevdoran commented Mar 16, 2018 • edited

For all changes:

For code changes:

For documentation related changes:

Note:

phrocker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevdoran Mar 19, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevdoran Mar 19, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevdoran Mar 19, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevdoran Mar 19, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevdoran Mar 19, 2018 • edited

Choose a reason for hiding this comment

kevdoran commented Mar 20, 2018

apiri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apiri commented Mar 22, 2018

apiri commented Mar 22, 2018

apiri commented Mar 22, 2018

kevdoran commented Mar 22, 2018

kevdoran commented Mar 16, 2018 •

edited

kevdoran Mar 19, 2018 •

edited

kevdoran Mar 19, 2018 •

edited

kevdoran Mar 19, 2018 •

edited

kevdoran Mar 19, 2018 •

edited

kevdoran Mar 19, 2018 •

edited