You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by MikeThomsen <gi...@git.apache.org> on 2018/03/07 17:07:04 UTC
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
GitHub user MikeThomsen opened a pull request:
https://github.com/apache/nifi/pull/2518
NIFI-4637 Added support for visibility labels to the HBase processors.
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-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 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/MikeThomsen/nifi NIFI-4637
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/2518.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 #2518
----
commit 78945e97191954915c9467d7d4db8d64d4d5f567
Author: Mike Thomsen <mi...@...>
Date: 2018-02-22T21:15:00Z
NIFI-4637 Added support for visibility labels to the HBase processors.
----
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184274827
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/ScanHBase.java ---
@@ -381,6 +384,7 @@ public void onTrigger(ProcessContext context, ProcessSession session) throws Pro
limitRows,
isReversed,
columns,
+ authorizations,
--- End diff --
Do you know if HBase scan API let ResultHander get the visibility label that is set to the cells retrieved by this scan? If so, do we want to include that in output contents? I think if HBase returns that, we can embed those at RowSerializer implementations such as JsonFullRowSerializer.
It can make scan -> delete flow easier.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r173482333
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -141,6 +149,16 @@
.allowableValues(NULL_FIELD_EMPTY, NULL_FIELD_SKIP)
.build();
+ protected static final PropertyDescriptor VISIBILITY_RECORD_PATH = new PropertyDescriptor.Builder()
+ .name("put-hb-rec-visibility-record-path")
+ .displayName("Visibility String Record Path Root")
+ .description("A record path that points to part of the record which contains a path to a mapping of visibility strings to record paths")
+ .required(false)
+ .addValidator(Validator.VALID) //new RecordPathPropertyNameValidator())
--- End diff --
Done
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184277181
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -142,6 +161,16 @@
.allowableValues(NULL_FIELD_EMPTY, NULL_FIELD_SKIP)
.build();
+ protected static final PropertyDescriptor VISIBILITY_RECORD_PATH = new PropertyDescriptor.Builder()
+ .name("put-hb-rec-visibility-record-path")
+ .displayName("Visibility String Record Path Root")
+ .description("A record path that points to part of the record which contains a path to a mapping of visibility strings to record paths")
+ .required(false)
+ .addValidator(Validator.VALID)
+ .build();
--- End diff --
I like this configuration. Should we add similar property to PutHBaseJson, in order to pass visibility label within the input JSON tree? Similar to `Column Family` property. How do you think?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184266796
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase-client-service-api/src/main/java/org/apache/nifi/hbase/HBaseClientService.java ---
@@ -127,6 +139,25 @@
void delete(String tableName, List<byte[]> rowIds) throws IOException;
+ /**
+ * Deletes a list of cells from HBase. This is intended to be used with granual delete operations.
+ *
+ * @param tableName the name of an HBase table.
+ * @param deletes a list of DeleteRequest objects.
+ * @throws IOException thrown when there are communication errors with HBase
+ */
+ void deleteCells(String tableName, List<DeleteRequest> deletes) throws IOException;
+
+ /**
+ * Deletes a list of rows in HBase. All cells are deleted.
--- End diff --
The 'All cells are deleted' should be updated to state that only matched cells are deleted if target cells have visibility label expression.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184726849
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase-client-service-api/src/main/java/org/apache/nifi/hbase/HBaseClientService.java ---
@@ -127,6 +139,25 @@
void delete(String tableName, List<byte[]> rowIds) throws IOException;
+ /**
+ * Deletes a list of cells from HBase. This is intended to be used with granual delete operations.
+ *
+ * @param tableName the name of an HBase table.
+ * @param deletes a list of DeleteRequest objects.
+ * @throws IOException thrown when there are communication errors with HBase
+ */
+ void deleteCells(String tableName, List<DeleteRequest> deletes) throws IOException;
+
+ /**
+ * Deletes a list of rows in HBase. All cells are deleted.
--- End diff --
done.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r172973696
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -194,6 +220,12 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
final String fieldEncodingStrategy = context.getProperty(FIELD_ENCODING_STRATEGY).getValue();
final String complexFieldStrategy = context.getProperty(COMPLEX_FIELD_STRATEGY).getValue();
final String rowEncodingStrategy = context.getProperty(ROW_ID_ENCODING_STRATEGY).getValue();
+ final String recordPathText = context.getProperty(VISIBILITY_RECORD_PATH).getValue();
+
+ RecordPath recordPath = null;
+ if (recordPathCache != null && recordPathText != null && !recordPathText.equals("")) {
--- End diff --
````java
if (recordPathCache != null && !StringUtils.isEmpty(recordPathText)) {
````
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r172973432
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -141,6 +149,16 @@
.allowableValues(NULL_FIELD_EMPTY, NULL_FIELD_SKIP)
.build();
+ protected static final PropertyDescriptor VISIBILITY_RECORD_PATH = new PropertyDescriptor.Builder()
+ .name("put-hb-rec-visibility-record-path")
+ .displayName("Visibility String Record Path Root")
+ .description("A record path that points to part of the record which contains a path to a mapping of visibility strings to record paths")
+ .required(false)
+ .addValidator(Validator.VALID) //new RecordPathPropertyNameValidator())
--- End diff --
Comment to remove?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r173495309
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase-client-service-api/src/main/java/org/apache/nifi/hbase/HBaseClientService.java ---
@@ -126,6 +138,25 @@
void delete(String tableName, List<byte[]> rowIds) throws IOException;
+ /**
+ * Deletes a list of cells from HBase. This is intended to be used with granual delete operations.
--- End diff --
Sorry, I was not arguing the wording, just mentioning a possible typo :)
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r172976209
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -349,18 +399,54 @@ public boolean checkAndPut(final String tableName, final byte[] rowId, final byt
@Override
public void delete(final String tableName, final byte[] rowId) throws IOException {
+ delete(tableName, rowId, null);
+ }
+
+ @Override
+ public void delete(String tableName, byte[] rowId, String visibilityLabel) throws IOException {
try (final Table table = connection.getTable(TableName.valueOf(tableName))) {
Delete delete = new Delete(rowId);
+ if (visibilityLabel != null && !visibilityLabel.trim().equals("")) {
+ delete.setCellVisibility(new CellVisibility(visibilityLabel));
+ }
table.delete(delete);
}
}
@Override
public void delete(String tableName, List<byte[]> rowIds) throws IOException {
+ delete(tableName, rowIds);
+ }
+
+ @Override
+ public void deleteCells(String tableName, List<DeleteRequest> deletes) throws IOException {
+ List<Delete> deleteRequests = new ArrayList<>();
+ for (int index = 0; index < deletes.size(); index++) {
+ DeleteRequest req = deletes.get(index);
+ Delete delete = new Delete(req.getRowId())
+ .addColumn(req.getColumnFamily(), req.getColumnQualifier());
+ if (req.getVisibilityLabel() != null && !req.getVisibilityLabel().trim().equals("")) {
--- End diff --
````StringUtils.isBlank(str)````
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r172975369
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase-client-service-api/src/main/java/org/apache/nifi/hbase/HBaseClientService.java ---
@@ -150,6 +194,40 @@
*/
void scan(String tableName, byte[] startRow, byte[] endRow, Collection<Column> columns, ResultHandler handler) throws IOException;
+ /**
+ * Scans the given table for the given rowId and passes the result to the handler.
+ *
+ * @param tableName the name of an HBase table to scan
+ * @param startRow the row identifier to start scanning at
+ * @param endRow the row identifier to end scanning at
+ * @param columns optional columns to return, if not specified all columns are returned
+ * @param visibilityLabels optional list of visibility labels that the user should be able to see when communicating with HBase
+ * @param handler a handler to process rows of the result
+ * @throws IOException thrown when there are communication errors with HBase
+ */
+ void scan(String tableName, byte[] startRow, byte[] endRow, Collection<Column> columns, List<String> visibilityLabels, ResultHandler handler) throws IOException;
+
+ /**
+ * Get all of the labels in HBase.
+ *
+ * @return a List of all of the labels.
+ */
+ List<String> getLabels();
+
+ /**
+ * Get all of the labels a given user can see.
+ * @param user the user to lookup.
+ * @return a List of all of the labels a user is allowed to see.
+ */
+ List<String> getLabelsForUser(String user);
+
+ /**
+ * Get all of the labels the current user (NiFi process user or Kerberos keytab principle) can see.
+ *
+ * @return a List of all of the labels the current can see.
--- End diff --
current *user*?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r173483953
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase-client-service-api/src/main/java/org/apache/nifi/hbase/HBaseClientService.java ---
@@ -126,6 +138,25 @@
void delete(String tableName, List<byte[]> rowIds) throws IOException;
+ /**
+ * Deletes a list of cells from HBase. This is intended to be used with granual delete operations.
--- End diff --
I think that's a reasonable word choice because deleting whole row is not necessarily possible all at once with visibility labels enabled. Each cell that has a label must have its visibility label sent as part of the delete request. So you have to build a nuanced delete request.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@joshelser Quick question about Accumulo, does it require you to specify the visibility string in a delete mutation?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by anoopsjohn <gi...@git.apache.org>.
Github user anoopsjohn commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r187593742
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -336,51 +346,85 @@ public void shutdown() {
}
}
+ private static final byte[] EMPTY_VIS_STRING;
+
+ static {
+ try {
+ EMPTY_VIS_STRING = "".getBytes("UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ private List<Put> buildPuts(byte[] rowKey, List<PutColumn> columns) {
+ List<Put> retVal = new ArrayList<>();
+
+ try {
+ Put put = null;
+
+ for (final PutColumn column : columns) {
--- End diff --
Allowing the different columns in same row to have diff visibility right. For each of the unique visibility one Put need to be there along with corresponding columns in it. I believe the below logic is doing that. Just saying.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r185746716
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -75,6 +83,17 @@
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ protected static final PropertyDescriptor DEFAULT_VISIBILITY_STRING = new PropertyDescriptor.Builder()
--- End diff --
Added that.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r182806554
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/test/java/org/apache/nifi/hbase/VisibilityIT.java ---
@@ -0,0 +1,510 @@
+/*
+ * 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.hbase;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.client.security.SecurityCapability;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.security.access.SecureTestUtil;
+import org.apache.hadoop.hbase.security.visibility.CellVisibility;
+import org.apache.hadoop.hbase.security.visibility.VisibilityClient;
+import org.apache.nifi.serialization.SimpleRecordSchema;
+import org.apache.nifi.serialization.record.MapRecord;
+import org.apache.nifi.serialization.record.MockRecordParser;
+import org.apache.nifi.serialization.record.RecordField;
+import org.apache.nifi.serialization.record.RecordFieldType;
+import org.apache.nifi.serialization.record.RecordSchema;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.security.PrivilegedExceptionAction;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+import static org.apache.hadoop.hbase.security.visibility.VisibilityConstants.LABELS_TABLE_NAME;
+
+public class VisibilityIT {
--- End diff --
I suppose we could do that since it's not like there's any real logic in the processors to test other than "fetch tokens and split them into a `List<String>`."
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/2518
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184737389
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -131,6 +133,59 @@ public void onScheduled(final ProcessContext context) {
clientService = context.getProperty(HBASE_CLIENT_SERVICE).asControllerService(HBaseClientService.class);
}
+ @Override
+ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
+ if (propertyDescriptorName.startsWith("visibility.")) {
+ String[] parts = propertyDescriptorName.split("\\.");
+ String displayName;
+ String description;
+
+ if (parts.length == 2) {
+ displayName = String.format("Column Family %s Default Visibility", parts[1]);
+ description = String.format("Default visibility setting for %s", parts[1]);
+ } else if (parts.length == 3) {
+ displayName = String.format("Column Qualifier %s.%s Default Visibility", parts[1], parts[2]);
+ description = String.format("Default visibility setting for %s.%s", parts[1], parts[2]);
+ } else {
+ return null;
+ }
+
+ return new PropertyDescriptor.Builder()
+ .name(propertyDescriptorName)
+ .displayName(displayName)
+ .description(description)
+ .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+ .dynamic(true)
+ .build();
--- End diff --
Done.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r178043080
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/pom.xml ---
@@ -79,6 +80,73 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-server</artifactId>
+ <version>1.3.1</version>
--- End diff --
FYI, 1.3.2 was just released if you want to pick that up :)
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@bbende @ijokarumawak could you take a look at this once 1.6 is released?
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@ijokarumawak I think it's all in now. I just finished the last change you requested.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r187531820
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/resources/META-INF/services/docs.org.apache.nifi.hbase.PutHBaseRecord/additionalDetails.html ---
@@ -0,0 +1,113 @@
+<!DOCTYPE html>
+<html lang="en">
+<!--
+ 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.
+-->
+<head>
+ <meta charset="utf-8" />
+ <title>PutHBaseRecord</title>
+ <link rel="stylesheet" href="../../../../../css/component-usage.css" type="text/css" />
+</head>
--- End diff --
These additional docs html file must be in a proper directory. which is `resources/docs/package.ClassName`. Currently, these HTMLs are not rendered at all.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@ijokarumawak pushed the documentation change you requested and switched PutHBaseCell/JSON to use dynamic properties.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184265359
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/DeleteHBaseCells.java ---
@@ -0,0 +1,139 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import org.apache.nifi.annotation.behavior.WritesAttribute;
+import org.apache.nifi.annotation.behavior.WritesAttributes;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Scanner;
+
+@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
+@WritesAttributes({
+ @WritesAttribute(attribute = "error.line", description = "The line number of the error."),
+ @WritesAttribute(attribute = "error.msg", description = "The message explaining the error.")
+})
+@Tags({"hbase", "delete", "cell", "cells", "visibility"})
+@CapabilityDescription("This processor allows the user to delete individual HBase cells by specifying one or more lines " +
+ "in the flowfile content that are a sequence composed of row ID, column family, column qualifier and associated visibility labels " +
+ "if visibility labels are enabled and in use. A user-defined separator is used to separate each of these pieces of data on each " +
+ "line, with :::: being the default separator.")
+public class DeleteHBaseCells extends AbstractDeleteHBase {
+ static final PropertyDescriptor SEPARATOR = new PropertyDescriptor.Builder()
+ .name("delete-hbase-cell-separator")
+ .displayName("Separator")
+ .description("Each line of the flowfile content is separated into components for building a delete using this" +
+ "separator. It should be something other than a single colon or a comma because these are values that " +
+ "are associated with columns and visibility labels respectively. To delete a row with ID xyz, column family abc, " +
+ "column qualifier def and visibility label PII&PHI, one would specify xyz::::abc::::def::::PII&PHI given the default " +
+ "value")
+ .required(true)
+ .defaultValue("::::")
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .build();
+
+ static final String ERROR_LINE = "error.line";
+ static final String ERROR_MSG = "error.msg";
+
+ @Override
+ protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
+ final List<PropertyDescriptor> properties = new ArrayList<>();
+ properties.add(HBASE_CLIENT_SERVICE);
+ properties.add(TABLE_NAME);
+ properties.add(SEPARATOR);
+
+ return properties;
+ }
+
+ private FlowFile writeErrorAttributes(int line, String msg, FlowFile file, ProcessSession session) {
+ file = session.putAttribute(file, ERROR_LINE, String.valueOf(line));
+ file = session.putAttribute(file, ERROR_MSG, msg != null ? msg : "");
+ return file;
+ }
+
+ private void logCell(String rowId, String family, String column, String visibility) {
+ StringBuilder sb = new StringBuilder()
+ .append("Assembling cell delete for...\t")
+ .append(String.format("Row ID: %s\t", rowId))
+ .append(String.format("Column Family: %s\t", family))
+ .append(String.format("Column Qualifier: %s\t", column))
+ .append(String.format("Visibility Label: %s", visibility));
+ getLogger().debug(sb.toString());
+ }
+
+ @Override
+ protected void doDelete(ProcessContext context, ProcessSession session) throws Exception {
+ FlowFile input = session.get();
+ if (input == null) {
+ return;
+ }
+
+ final String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(input).getValue();
+ final String tableName = context.getProperty(TABLE_NAME).evaluateAttributeExpressions(input).getValue();
+ try (InputStream is = session.read(input)) {
+ Scanner scanner = new Scanner(is);
+ int lineNum = 1;
+ List<DeleteRequest> deletes = new ArrayList<>();
+ while (scanner.hasNextLine()) {
+ String line = scanner.nextLine().trim();
+ if (line.equals("")) {
+ continue;
+ }
+ String[] parts = line.split(separator);
+ if (parts.length < 3 || parts.length > 4) {
+ final String msg = String.format("Invalid line length. It must have 3 or 4 components. It had %d.", parts.length);
+ input = writeErrorAttributes(lineNum, msg, input, session);
+ session.transfer(input, REL_FAILURE);
+ getLogger().error(msg);
+ return;
+ }
+ String rowId = parts[0];
+ String family = parts[1];
+ String column = parts[2];
+ String visibility = parts.length == 4 ? parts[3] : null;
+
+ DeleteRequest request = new DeleteRequest(rowId.getBytes(), family.getBytes(), column.getBytes(), visibility);
+ deletes.add(request);
+
+ if (getLogger().isDebugEnabled()) {
+ logCell(rowId, family, column, visibility);
+ }
+
+ lineNum++;
+ }
+ is.close();
+ clientService.deleteCells(tableName, deletes);
+
+ session.transfer(input, REL_SUCCESS);
+ } catch (Exception ex) {
+ input = writeErrorAttributes(-1, ex.getMessage(), input, session);
--- End diff --
Is it possible to move the lineNum declaration before try statement so that this catch block can use the line number?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r178043335
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/pom.xml ---
@@ -79,6 +80,73 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-server</artifactId>
+ <version>1.3.1</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-server</artifactId>
+ <version>1.3.1</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-hdfs</artifactId>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-hdfs</artifactId>
+ <type>test-jar</type>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-common</artifactId>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-common</artifactId>
+ <version>2.9.0</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-auth</artifactId>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-hadoop-compat</artifactId>
+ <version>1.3.1</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-hadoop2-compat</artifactId>
+ <version>1.3.1</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-hbase_1_1_2-client-service</artifactId>
+ <version>1.6.0-SNAPSHOT</version>
--- End diff --
Ideally, there's a parent pom with all other nifi modules listed to avoid the need to duplicate the version here in the "child" pom.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184912296
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -75,6 +83,17 @@
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ protected static final PropertyDescriptor DEFAULT_VISIBILITY_STRING = new PropertyDescriptor.Builder()
--- End diff --
That's true using `pickVisibilityString` will add another complexity. However it can reduce the size of incoming Record formatted FlowFile.
My understanding is that currently each record needs to have the map field in order to use visibility labels even if there are 1000 records requiring the same visibility labels for cells in each record. In order to add such map field, user might have to do schema conversion and enrichment process before passing FlowFile to PutHBaseRecord processor. Having a smart (yet a bit complex necessarily) default mechanism would be helpful I think.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
I'll add a patch to revert the version # on the client lib. Since HDP is on 1.1.2 and CDH 5.14 is still on 1.2, I don't think it's safe to jump up to anything higher than a 1.2 client anyway.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2518
@MikeThomsen Thanks for the updates. It mostly looks good. The only concern I have is the gap between the name of NAR and the HBase client version. One possible solution is to rename the NAR from `nifi-hbase_1_1_2-client-service` to `nifi-hbase_1_1_x-client-service` or `nifi-hbase_1_1-client-service`, to avoid having too specific version in it so that we can update it in the future, assuming NiFi can pick the right NAR for existing flow when user upgrade NiFi even nar name changes.
If you agree with that approach, or to keep discussion on that specific topic separately, would you revert the HBase dependency version back to 1.1.2 for this PR, and submit another JIRA to do the version bump? Then we can close this one.
@bbende Any comment from you on this subject would be appreciated.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r172975481
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase-client-service-api/src/main/java/org/apache/nifi/hbase/HBaseClientService.java ---
@@ -126,6 +138,25 @@
void delete(String tableName, List<byte[]> rowIds) throws IOException;
+ /**
+ * Deletes a list of cells from HBase. This is intended to be used with granual delete operations.
--- End diff --
granular?
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/nifi/pull/2518
> Would you be open to a code review in the future (months away at least) if I were to start adding Accumulo support?
Feel free to hit me up :)
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r183990982
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/DeleteHBaseCells.java ---
@@ -0,0 +1,130 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import org.apache.nifi.annotation.behavior.WritesAttribute;
+import org.apache.nifi.annotation.behavior.WritesAttributes;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Scanner;
+
+@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
+@WritesAttributes({
+ @WritesAttribute(attribute = "error.line", description = "The line number of the error."),
+ @WritesAttribute(attribute = "error.msg", description = "The message explaining the error.")
+})
+public class DeleteHBaseCells extends AbstractDeleteHBase {
--- End diff --
This class should provide more documentation on what it does, tags for user to search, and what expected FlowFile format looks like ... etc
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184276347
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseCell.java ---
@@ -82,6 +79,8 @@ protected PutFlowFile createPut(final ProcessSession session, final ProcessConte
final String columnQualifier = context.getProperty(COLUMN_QUALIFIER).evaluateAttributeExpressions(flowFile).getValue();
final String timestampValue = context.getProperty(TIMESTAMP).evaluateAttributeExpressions(flowFile).getValue();
+ final String visibilityStringToUse = pickVisibilityString(columnFamily, columnQualifier, flowFile, context);
--- End diff --
Since PutHBaseCell is mutating only one cell, it is more intuitive to provide a configuration property directly specifying a visibility label expression, IN ADDITION to the default ones using dynamic properties. Thoughts?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184734171
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -131,6 +133,59 @@ public void onScheduled(final ProcessContext context) {
clientService = context.getProperty(HBASE_CLIENT_SERVICE).asControllerService(HBaseClientService.class);
}
+ @Override
+ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
+ if (propertyDescriptorName.startsWith("visibility.")) {
+ String[] parts = propertyDescriptorName.split("\\.");
+ String displayName;
+ String description;
+
+ if (parts.length == 2) {
+ displayName = String.format("Column Family %s Default Visibility", parts[1]);
+ description = String.format("Default visibility setting for %s", parts[1]);
+ } else if (parts.length == 3) {
+ displayName = String.format("Column Qualifier %s.%s Default Visibility", parts[1], parts[2]);
+ description = String.format("Default visibility setting for %s.%s", parts[1], parts[2]);
+ } else {
+ return null;
+ }
+
+ return new PropertyDescriptor.Builder()
+ .name(propertyDescriptorName)
+ .displayName(displayName)
+ .description(description)
+ .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+ .dynamic(true)
+ .build();
+ }
+
+ return null;
+ }
+
+ protected String pickVisibilityString(String columnFamily, String columnQualifier, FlowFile flowFile, ProcessContext context) {
+ final String lookupKey = String.format("visibility.%s.%s", columnFamily, columnQualifier);
+ final String fromAttribute = flowFile.getAttribute(lookupKey);
+ if (fromAttribute != null) {
+ return fromAttribute;
+ } else {
+ PropertyValue descriptor = context.getProperty(lookupKey);
+ if (descriptor == null) {
+ descriptor = context.getProperty(String.format("visibility.%s", columnFamily));
+ }
+
+ String retVal = descriptor != null ? descriptor.getValue() : null;
+
+ return retVal;
+ }
+ }
+
+ protected String pickVisibilityString(String defaultVisibilityString, String columnFamily, String columnQualifier, FlowFile flowFile) {
--- End diff --
Done.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184727040
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/pom.xml ---
@@ -25,7 +25,7 @@
<artifactId>nifi-hbase_1_1_2-client-service</artifactId>
<packaging>jar</packaging>
<properties>
- <hbase.version>1.1.2</hbase.version>
+ <hbase.version>1.1.13</hbase.version>
--- End diff --
@bbende if you think it is, I'll revert back to 1.1.2 in a separate commit. 1.1.13 is the last stable client for 1.1.X that I can find on maven central.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
Added `additionalDetails` for `PutHBaseCell` and `PutHBaseRow`. Also fixed up some of the behavior of the `pickVisibilityLabel` method.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184727554
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -336,51 +348,86 @@ public void shutdown() {
}
}
+ private static final byte[] EMPTY_VIS_STRING;
+
+ static {
+ try {
+ EMPTY_VIS_STRING = "".getBytes("UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ private List<Put> buildPuts(byte[] rowKey, List<PutColumn> columns) {
+ List<Put> retVal = new ArrayList<>();
+
+ try {
+ Put put = null;
+
+ for (final PutColumn column : columns) {
+ if (put == null || (put.getCellVisibility() == null && column.getVisibility() != null) || ( put.getCellVisibility() != null
+ && !put.getCellVisibility().getExpression().equals(column.getVisibility())
+ )) {
+ put = new Put(rowKey);
+
+ if (column.getVisibility() != null) {
+ put.setCellVisibility(new CellVisibility(column.getVisibility()));
+ }
+ retVal.add(put);
+ }
+
+ if (column.getTimestamp() != null) {
+ put.addColumn(
+ column.getColumnFamily(),
+ column.getColumnQualifier(),
+ column.getTimestamp(),
+ column.getBuffer());
+ } else {
+ put.addColumn(
+ column.getColumnFamily(),
+ column.getColumnQualifier(),
+ column.getBuffer());
+ }
+ }
+ } catch (DeserializationException de) {
+ getLogger().error("Error writing cell visibility statement.", de);
+ throw new RuntimeException(de);
+ }
+
+ return retVal;
+ }
+
@Override
public void put(final String tableName, final Collection<PutFlowFile> puts) throws IOException {
try (final Table table = connection.getTable(TableName.valueOf(tableName))) {
// Create one Put per row....
final Map<String, Put> rowPuts = new HashMap<>();
+ final Map<String, List<PutColumn>> sorted = new HashMap<>();
+ final List<Put> newPuts = new ArrayList<>();
+
for (final PutFlowFile putFlowFile : puts) {
- //this is used for the map key as a byte[] does not work as a key.
final String rowKeyString = new String(putFlowFile.getRow(), StandardCharsets.UTF_8);
- Put put = rowPuts.get(rowKeyString);
- if (put == null) {
- put = new Put(putFlowFile.getRow());
- rowPuts.put(rowKeyString, put);
+ List<PutColumn> columns = sorted.get(rowKeyString);
+ if (columns == null) {
+ columns = new ArrayList<>();
+ sorted.put(rowKeyString, columns);
}
- for (final PutColumn column : putFlowFile.getColumns()) {
- if (column.getTimestamp() != null) {
- put.addColumn(
- column.getColumnFamily(),
- column.getColumnQualifier(),
- column.getTimestamp(),
- column.getBuffer());
- } else {
- put.addColumn(
- column.getColumnFamily(),
- column.getColumnQualifier(),
- column.getBuffer());
- }
- }
+ columns.addAll(putFlowFile.getColumns());
+ }
+
+ for (final Map.Entry<String, List<PutColumn>> entry : sorted.entrySet()) {
+ newPuts.addAll(buildPuts(entry.getKey().getBytes(StandardCharsets.UTF_8), entry.getValue()));
}
- table.put(new ArrayList<>(rowPuts.values()));
+ table.put(new ArrayList<>(newPuts)); /*rowPuts.values()));*/
--- End diff --
Done. Not even sure why I did that...
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r183030677
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/test/java/org/apache/nifi/hbase/VisibilityIT.java ---
@@ -0,0 +1,510 @@
+/*
+ * 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.hbase;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.client.security.SecurityCapability;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.security.access.SecureTestUtil;
+import org.apache.hadoop.hbase.security.visibility.CellVisibility;
+import org.apache.hadoop.hbase.security.visibility.VisibilityClient;
+import org.apache.nifi.serialization.SimpleRecordSchema;
+import org.apache.nifi.serialization.record.MapRecord;
+import org.apache.nifi.serialization.record.MockRecordParser;
+import org.apache.nifi.serialization.record.RecordField;
+import org.apache.nifi.serialization.record.RecordFieldType;
+import org.apache.nifi.serialization.record.RecordSchema;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.security.PrivilegedExceptionAction;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+import static org.apache.hadoop.hbase.security.visibility.VisibilityConstants.LABELS_TABLE_NAME;
+
+public class VisibilityIT {
--- End diff --
I got to thinking about it last night and realized that there really isn't much value in having an integration test because what it's really doing is just testing whether or not hbase client and server are behaving themselves.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2518
@MikeThomsen Thanks for rebasing. I am now testing against my HBase instance.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184911380
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -142,6 +161,16 @@
.allowableValues(NULL_FIELD_EMPTY, NULL_FIELD_SKIP)
.build();
+ protected static final PropertyDescriptor VISIBILITY_RECORD_PATH = new PropertyDescriptor.Builder()
+ .name("put-hb-rec-visibility-record-path")
+ .displayName("Visibility String Record Path Root")
+ .description("A record path that points to part of the record which contains a path to a mapping of visibility strings to record paths")
+ .required(false)
+ .addValidator(Validator.VALID)
+ .build();
--- End diff --
I agree with that. If people have JSON input and want to specify visibility labels like that, they can use PutHBaseRecord. Thanks, no change needed.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184732693
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -142,6 +161,16 @@
.allowableValues(NULL_FIELD_EMPTY, NULL_FIELD_SKIP)
.build();
+ protected static final PropertyDescriptor VISIBILITY_RECORD_PATH = new PropertyDescriptor.Builder()
+ .name("put-hb-rec-visibility-record-path")
+ .displayName("Visibility String Record Path Root")
+ .description("A record path that points to part of the record which contains a path to a mapping of visibility strings to record paths")
+ .required(false)
+ .addValidator(Validator.VALID)
+ .build();
--- End diff --
I'm not sure that'd be a good idea. Might discourage people from using the record version in some cases.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2518
@MikeThomsen For upgrading HBase dependency version, I'd do create a new NAR. So that people have dependency with HBase 1.1.x can keep using HBase_1_1_2_ClientService. We do the same to support different Kafka versions.
About the naming, 'labels' or 'authorizations', I added [another comment above](#discussion_r182954434). Please check that out.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r178304323
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -63,6 +64,17 @@
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ protected static final PropertyDescriptor DEFAULT_VISIBILITY_STRING = new PropertyDescriptor.Builder()
+ .name("hbase-default-vis-string")
+ .displayName("Default Visibility String")
+ .description("When using visibility labels, any value set in this field will be applied to all cells that are written unless " +
--- End diff --
That's a very good point.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2518
@bbende Thanks for your comment. That's true, we need to rename not only NAR file but also component class names. I agree with staying 1.1.2 unless there is no necessary updates.
@MikeThomsen The latest Travis failed to build, but the reason was timeout. I'm going to build and test locally once again just in case now. If it looks fine, then I will squash commits and merge.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@ijokarumawak forgot the documentation...
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2518
Forgot to mention that the NiFi Flow I used to test, is available here:
https://gist.github.com/ijokarumawak/c455075f7830935189e5efbd83e5d863
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184277876
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -368,9 +412,18 @@ protected PutFlowFile createPut(ProcessContext context, Record record, RecordSch
timestamp = null;
}
+ RecordField visField = null;
+ Map visSettings = null;
+ if (recordPath != null) {
+ final RecordPathResult result = recordPath.evaluate(record);
+ FieldValue fv = result.getSelectedFields().findFirst().get();
+ visField = fv.getField();
+ visSettings = (Map)fv.getValue();
+ }
+
List<PutColumn> columns = new ArrayList<>();
for (String name : schema.getFieldNames()) {
- if (name.equals(rowFieldName) || name.equals(timestampFieldName)) {
+ if (name.equals(rowFieldName) || name.equals(timestampFieldName) || (visField != null && name.equals(visField.getFieldName()))) {
--- End diff --
Good stuff, I'd expect PutHBaseJson has the same capability.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r172977970
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/GetHBase.java ---
@@ -16,28 +16,6 @@
*/
package org.apache.nifi.hbase;
-import java.io.File;
--- End diff --
Just a detail: could you revert the import order changes?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r178044849
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -63,6 +64,17 @@
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ protected static final PropertyDescriptor DEFAULT_VISIBILITY_STRING = new PropertyDescriptor.Builder()
+ .name("hbase-default-vis-string")
+ .displayName("Default Visibility String")
+ .description("When using visibility labels, any value set in this field will be applied to all cells that are written unless " +
--- End diff --
It might be worth mentioning that the HBase installation that you're trying to write these too must have the VisibilityCoprocessor already loaded. Unsuspecting users might just assume that nifi automagically did something to HBase to let it handle the labels.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@bbende @ijokarumawak To move that IT into the client test package, I have to upgrade the client's hbase-client dependency to 1.2.X from 1.1.X. Do you see anything wrong with doing that? At this point 1.1.2 is very old.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2518
@MikeThomsen Thanks for the quick update, I will continue reviewing.
@bbende Thanks for the info. What I was wondering about is why HBase requires a 'Visibility Label Expression' instead of a comma separated visibility label list for delete operations.
In order to understand the theme more, I installed and played with Accumulo, too. It turned out Accumulo and HBase both require 'Visibility Label Expression' for delete.
@joshelser Would you please educate me why delete operation uses expression instead of a comma separated labels as scan does? E.g. to delete a cell having `(apple&carrot)|broccoli|spinach`, I had to specify exactly the same expression. I expected executing a delete request by a user having all of authorizations would delete the cell, or the logical expression would match if `spinach` was specified. But it didn't behave like that, a matching expression was needed.
Following results are what I got executing delete operations based on the [Accumulo example](https://accumulo.apache.org/1.7/examples/visibility):
```
# Insert test data
username@instance vistest> insert row f1 q1 v1 -l A
username@instance vistest> insert row f2 q2 v2 -l A&B
username@instance vistest> insert row f3 q3 v3 -l (apple&carrot)|broccoli|spinach
# It seems users authorizations are not used for delete, and need to be specified explicitly
username@instance vistest> delete row f1 q1
username@instance vistest> scan
row f1:q1 [A] v1
row f2:q2 [A&B] v2
row f3:q3 [(apple&carrot)|broccoli|spinach] v3
username@instance vistest> delete row f1 q1 -l A
username@instance vistest> scan
row f2:q2 [A&B] v2
row f3:q3 [(apple&carrot)|broccoli|spinach] v3
# Specifying wrong label does not return error
username@instance vistest> delete row f3 q3 -l spina
username@instance vistest> scan
row f3:q3 [(apple&carrot)|broccoli|spinach] v3
# The visibility label expression should match exactly
username@instance vistest> delete row f3 q3 -l spinach
username@instance vistest> scan
row f3:q3 [(apple&carrot)|broccoli|spinach] v3
username@instance vistest> delete row f3 q3 -l spinach|broccoli
username@instance vistest> scan
row f3:q3 [(apple&carrot)|broccoli|spinach] v3
username@instance vistest> delete row f3 q3 -l spinach|broccoli|(apple&carrot)
username@instance vistest> scan
row f3:q3 [(apple&carrot)|broccoli|spinach] v3
username@instance vistest> delete row f3 q3 -l (apple&carrot)|broccoli
username@instance vistest> scan
row f3:q3 [(apple&carrot)|broccoli|spinach] v3
username@instance vistest> delete row f3 q3 -l (apple&carrot)|broccoli|spinach
username@instance vistest> scan
# There are no rows
username@instance vistest>
```
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184733869
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseCell.java ---
@@ -82,6 +79,8 @@ protected PutFlowFile createPut(final ProcessSession session, final ProcessConte
final String columnQualifier = context.getProperty(COLUMN_QUALIFIER).evaluateAttributeExpressions(flowFile).getValue();
final String timestampValue = context.getProperty(TIMESTAMP).evaluateAttributeExpressions(flowFile).getValue();
+ final String visibilityStringToUse = pickVisibilityString(columnFamily, columnQualifier, flowFile, context);
--- End diff --
I'm not sure what you mean here. Can you expand on that?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184729882
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -131,6 +133,59 @@ public void onScheduled(final ProcessContext context) {
clientService = context.getProperty(HBASE_CLIENT_SERVICE).asControllerService(HBaseClientService.class);
}
+ @Override
+ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
--- End diff --
Done.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r178483377
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/pom.xml ---
@@ -79,6 +80,73 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-server</artifactId>
+ <version>1.3.1</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-server</artifactId>
+ <version>1.3.1</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-hdfs</artifactId>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-hdfs</artifactId>
+ <type>test-jar</type>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-common</artifactId>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-common</artifactId>
+ <version>2.9.0</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-auth</artifactId>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-hadoop-compat</artifactId>
+ <version>1.3.1</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-hadoop2-compat</artifactId>
+ <version>1.3.1</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-hbase_1_1_2-client-service</artifactId>
+ <version>1.6.0-SNAPSHOT</version>
--- End diff --
What I'm used to seeing is that, in dependencyManagement for the appropriate parent pom (in this case, https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-standard-services/pom.xml), this GAV would be declared which would turn this local GAV into just GA.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184278583
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -75,6 +83,17 @@
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ protected static final PropertyDescriptor DEFAULT_VISIBILITY_STRING = new PropertyDescriptor.Builder()
--- End diff --
Is there any reason to not use `pickVisibiliyString` at PutHBaseRecord?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184733625
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -142,6 +161,16 @@
.allowableValues(NULL_FIELD_EMPTY, NULL_FIELD_SKIP)
.build();
+ protected static final PropertyDescriptor VISIBILITY_RECORD_PATH = new PropertyDescriptor.Builder()
+ .name("put-hb-rec-visibility-record-path")
+ .displayName("Visibility String Record Path Root")
+ .description("A record path that points to part of the record which contains a path to a mapping of visibility strings to record paths")
+ .required(false)
+ .addValidator(Validator.VALID)
+ .build();
--- End diff --
I agree.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2518
@MikeThomsen @bbende HBase allows user to set visibility label expression using labels those are not associated to the user by default. There is a server side configuration to strict this `hbase.security.visibility.mutations.checkauths`.
I am not familiar with Accumulo behavior but there was a mention about the difference between Accumulo and HBase.
> The deletes happen based on pattern matching. If suppose a Cell has SECRET&TOPSECRET as the visibility expression then any delete coming with the same expression is matched for, but TOPSECRET&SECRET is also considered to be same
> A&B<=>B&A
> A|B<=>B|A.
> This is the difference between what Accumulo does in terms of delete where only the exact match is looked out for.
https://issues.apache.org/jira/browse/HBASE-10885?focusedCommentId=14053522&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14053522
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r173482943
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase-client-service-api/src/main/java/org/apache/nifi/hbase/HBaseClientService.java ---
@@ -150,6 +194,40 @@
*/
void scan(String tableName, byte[] startRow, byte[] endRow, Collection<Column> columns, ResultHandler handler) throws IOException;
+ /**
+ * Scans the given table for the given rowId and passes the result to the handler.
+ *
+ * @param tableName the name of an HBase table to scan
+ * @param startRow the row identifier to start scanning at
+ * @param endRow the row identifier to end scanning at
+ * @param columns optional columns to return, if not specified all columns are returned
+ * @param visibilityLabels optional list of visibility labels that the user should be able to see when communicating with HBase
+ * @param handler a handler to process rows of the result
+ * @throws IOException thrown when there are communication errors with HBase
+ */
+ void scan(String tableName, byte[] startRow, byte[] endRow, Collection<Column> columns, List<String> visibilityLabels, ResultHandler handler) throws IOException;
+
+ /**
+ * Get all of the labels in HBase.
+ *
+ * @return a List of all of the labels.
+ */
+ List<String> getLabels();
+
+ /**
+ * Get all of the labels a given user can see.
+ * @param user the user to lookup.
+ * @return a List of all of the labels a user is allowed to see.
+ */
+ List<String> getLabelsForUser(String user);
+
+ /**
+ * Get all of the labels the current user (NiFi process user or Kerberos keytab principle) can see.
+ *
+ * @return a List of all of the labels the current can see.
--- End diff --
I have to give this some more thought, but HBase visibility labels work with or without a Kerberos configuration. So without it, the HBase client does simple auth and sends over the user running NiFi as the user.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r183994650
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -64,6 +65,18 @@
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ protected static final PropertyDescriptor DEFAULT_VISIBILITY_STRING = new PropertyDescriptor.Builder()
+ .name("hbase-default-vis-string")
+ .displayName("Default Visibility String")
+ .description("When using visibility labels, any value set in this field will be applied to all cells that are written unless " +
+ "an attribute with the convention \"visibility.COLUMN_FAMILY.COLUMN_QUALIFIER\" is present on the flowfile. If this field " +
+ "is left blank, it will be assumed that no visibility is to be set unless visibility-related attributes are set. NOTE: " +
+ "this configuration will have no effect on your data if you have not enabled visibility labels in the HBase cluster.")
--- End diff --
If user would like to set visibility label per column family, how would they do that? Default visibility string may be too broad but setting attributes for each column qualifier can be error-prone as those configuration need to be updated as new qualifier emerges. Isn't it beneficial to support column family level default visibility? Probably by user defined dynamic property can be used.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184732094
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -131,6 +133,59 @@ public void onScheduled(final ProcessContext context) {
clientService = context.getProperty(HBASE_CLIENT_SERVICE).asControllerService(HBaseClientService.class);
}
+ @Override
+ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
+ if (propertyDescriptorName.startsWith("visibility.")) {
+ String[] parts = propertyDescriptorName.split("\\.");
+ String displayName;
+ String description;
+
+ if (parts.length == 2) {
+ displayName = String.format("Column Family %s Default Visibility", parts[1]);
+ description = String.format("Default visibility setting for %s", parts[1]);
+ } else if (parts.length == 3) {
+ displayName = String.format("Column Qualifier %s.%s Default Visibility", parts[1], parts[2]);
+ description = String.format("Default visibility setting for %s.%s", parts[1], parts[2]);
+ } else {
+ return null;
+ }
+
+ return new PropertyDescriptor.Builder()
+ .name(propertyDescriptorName)
+ .displayName(displayName)
+ .description(description)
+ .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+ .dynamic(true)
+ .build();
+ }
+
+ return null;
+ }
+
+ protected String pickVisibilityString(String columnFamily, String columnQualifier, FlowFile flowFile, ProcessContext context) {
+ final String lookupKey = String.format("visibility.%s.%s", columnFamily, columnQualifier);
--- End diff --
I think that's a valid point, fixing...
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184911299
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseCell.java ---
@@ -82,6 +79,8 @@ protected PutFlowFile createPut(final ProcessSession session, final ProcessConte
final String columnQualifier = context.getProperty(COLUMN_QUALIFIER).evaluateAttributeExpressions(flowFile).getValue();
final String timestampValue = context.getProperty(TIMESTAMP).evaluateAttributeExpressions(flowFile).getValue();
+ final String visibilityStringToUse = pickVisibilityString(columnFamily, columnQualifier, flowFile, context);
--- End diff --
When I was configured the processor like below:
- Row Identifier: ${hbase.rowid}
- Column Family: ${hbase.column.family}
- Column Qualifier: ${hbase.column.qualifier}
I was looking for a property for visibility expression to specify EL similarly, e.g. '${hbase.visibility}'. Currently, in order to add visibility, a FlowFile must have an attribute named 'visibility.family.qualifier', or a dynamic property is needed. I think I wanted to put the configuration directory without adding a dynamic property. But it should be fine since we now have a detailed docs. Thanks, no change is needed for the comment.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2518
@joshelser Thanks for your prompt answer. I didn't know that different values can be stored with the same key but different visibility label expression. The explanation 'The expression also contributes to the uniqueness of that key, almost acting as a kind of "attribute" for the record being store.' makes current design more understandable for me. If that is a part of key for the value, then perfect matching one should be specified. I will experiment it later. Thanks!
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r182619764
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/VisibilityFetchSupport.java ---
@@ -0,0 +1,53 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.util.StringUtils;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public interface VisibilityFetchSupport {
+ PropertyDescriptor AUTHORIZATIONS = new PropertyDescriptor.Builder()
+ .name("hbase-fetch-row-authorizations")
+ .displayName("Authorizations")
+ .description("The list of authorizations to pass to the scanner. This will be ignored if cell visibility labels are not in use.")
--- End diff --
The labels are used for authorizations certainly, but they're used to filter specific cells, too, right? E.g. if tag 'A' and tag 'B' are associated with a user, then the user can access following cells:
- Tagged with 'A'
- Tagged with 'A' and 'B'
- Tagged with 'B'
In that case, if NiFi user would like to fetch/scan only cells tagged with 'B', the name of the property 'Authorizations' does not sound perfect to me. It would make more sense if we called it simply 'Visibility Labels'. How do you think?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184731267
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/DeleteHBaseCells.java ---
@@ -0,0 +1,139 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import org.apache.nifi.annotation.behavior.WritesAttribute;
+import org.apache.nifi.annotation.behavior.WritesAttributes;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Scanner;
+
+@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
+@WritesAttributes({
+ @WritesAttribute(attribute = "error.line", description = "The line number of the error."),
+ @WritesAttribute(attribute = "error.msg", description = "The message explaining the error.")
+})
+@Tags({"hbase", "delete", "cell", "cells", "visibility"})
+@CapabilityDescription("This processor allows the user to delete individual HBase cells by specifying one or more lines " +
+ "in the flowfile content that are a sequence composed of row ID, column family, column qualifier and associated visibility labels " +
+ "if visibility labels are enabled and in use. A user-defined separator is used to separate each of these pieces of data on each " +
+ "line, with :::: being the default separator.")
+public class DeleteHBaseCells extends AbstractDeleteHBase {
+ static final PropertyDescriptor SEPARATOR = new PropertyDescriptor.Builder()
+ .name("delete-hbase-cell-separator")
+ .displayName("Separator")
+ .description("Each line of the flowfile content is separated into components for building a delete using this" +
+ "separator. It should be something other than a single colon or a comma because these are values that " +
+ "are associated with columns and visibility labels respectively. To delete a row with ID xyz, column family abc, " +
+ "column qualifier def and visibility label PII&PHI, one would specify xyz::::abc::::def::::PII&PHI given the default " +
+ "value")
+ .required(true)
+ .defaultValue("::::")
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .build();
+
+ static final String ERROR_LINE = "error.line";
+ static final String ERROR_MSG = "error.msg";
+
+ @Override
+ protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
+ final List<PropertyDescriptor> properties = new ArrayList<>();
+ properties.add(HBASE_CLIENT_SERVICE);
+ properties.add(TABLE_NAME);
+ properties.add(SEPARATOR);
+
+ return properties;
+ }
+
+ private FlowFile writeErrorAttributes(int line, String msg, FlowFile file, ProcessSession session) {
+ file = session.putAttribute(file, ERROR_LINE, String.valueOf(line));
+ file = session.putAttribute(file, ERROR_MSG, msg != null ? msg : "");
+ return file;
+ }
+
+ private void logCell(String rowId, String family, String column, String visibility) {
+ StringBuilder sb = new StringBuilder()
+ .append("Assembling cell delete for...\t")
+ .append(String.format("Row ID: %s\t", rowId))
+ .append(String.format("Column Family: %s\t", family))
+ .append(String.format("Column Qualifier: %s\t", column))
+ .append(String.format("Visibility Label: %s", visibility));
+ getLogger().debug(sb.toString());
+ }
+
+ @Override
+ protected void doDelete(ProcessContext context, ProcessSession session) throws Exception {
+ FlowFile input = session.get();
+ if (input == null) {
+ return;
+ }
+
+ final String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(input).getValue();
+ final String tableName = context.getProperty(TABLE_NAME).evaluateAttributeExpressions(input).getValue();
+ try (InputStream is = session.read(input)) {
+ Scanner scanner = new Scanner(is);
+ int lineNum = 1;
+ List<DeleteRequest> deletes = new ArrayList<>();
+ while (scanner.hasNextLine()) {
+ String line = scanner.nextLine().trim();
+ if (line.equals("")) {
+ continue;
+ }
+ String[] parts = line.split(separator);
+ if (parts.length < 3 || parts.length > 4) {
+ final String msg = String.format("Invalid line length. It must have 3 or 4 components. It had %d.", parts.length);
+ input = writeErrorAttributes(lineNum, msg, input, session);
+ session.transfer(input, REL_FAILURE);
+ getLogger().error(msg);
+ return;
+ }
+ String rowId = parts[0];
+ String family = parts[1];
+ String column = parts[2];
+ String visibility = parts.length == 4 ? parts[3] : null;
+
+ DeleteRequest request = new DeleteRequest(rowId.getBytes(), family.getBytes(), column.getBytes(), visibility);
+ deletes.add(request);
+
+ if (getLogger().isDebugEnabled()) {
+ logCell(rowId, family, column, visibility);
+ }
+
+ lineNum++;
+ }
+ is.close();
+ clientService.deleteCells(tableName, deletes);
--- End diff --
Done.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
Had to rebase because of the commit that added `ScanHBase`. That doesn't have visibility labels added to it yet. That needs to be done. Keeping this ticket open because there's plenty that can be reviewed in the mean time if anyone is interested.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184287966
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -131,6 +133,59 @@ public void onScheduled(final ProcessContext context) {
clientService = context.getProperty(HBASE_CLIENT_SERVICE).asControllerService(HBaseClientService.class);
}
+ @Override
+ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
+ if (propertyDescriptorName.startsWith("visibility.")) {
+ String[] parts = propertyDescriptorName.split("\\.");
+ String displayName;
+ String description;
+
+ if (parts.length == 2) {
+ displayName = String.format("Column Family %s Default Visibility", parts[1]);
+ description = String.format("Default visibility setting for %s", parts[1]);
+ } else if (parts.length == 3) {
+ displayName = String.format("Column Qualifier %s.%s Default Visibility", parts[1], parts[2]);
+ description = String.format("Default visibility setting for %s.%s", parts[1], parts[2]);
+ } else {
+ return null;
+ }
+
+ return new PropertyDescriptor.Builder()
+ .name(propertyDescriptorName)
+ .displayName(displayName)
+ .description(description)
+ .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+ .dynamic(true)
+ .build();
+ }
+
+ return null;
+ }
+
+ protected String pickVisibilityString(String columnFamily, String columnQualifier, FlowFile flowFile, ProcessContext context) {
+ final String lookupKey = String.format("visibility.%s.%s", columnFamily, columnQualifier);
+ final String fromAttribute = flowFile.getAttribute(lookupKey);
+ if (fromAttribute != null) {
+ return fromAttribute;
+ } else {
+ PropertyValue descriptor = context.getProperty(lookupKey);
+ if (descriptor == null) {
--- End diff --
This condition does not work as expected. Because even if there is only 'visibility.family' defined, `context.getProperty(visibility.family.qualifier)` returns a non-null object which having its value null. Please change this to `(descriptor == null || !descriptor.isSet())`. Also please add some unit tests.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r183998999
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/VisibilityFetchSupport.java ---
@@ -0,0 +1,53 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.util.StringUtils;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public interface VisibilityFetchSupport {
+ PropertyDescriptor AUTHORIZATIONS = new PropertyDescriptor.Builder()
+ .name("hbase-fetch-row-authorizations")
+ .displayName("Authorizations")
+ .description("The list of authorizations to pass to the scanner. This will be ignored if cell visibility labels are not in use.")
--- End diff --
I agree on drawing a firm distinction between those, I think it just a matter of naming. Especially it was ambiguous to me where I am supposed to sed 'A set of Visibility Labels' meaning just a comma separated list of labels and 'Visibility Expression', a logical formula to verify users accessibility.
I will comment on processor properties to propose more specific suggestions later. Thanks.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r182788174
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase-client-service-api/src/main/java/org/apache/nifi/hbase/HBaseClientService.java ---
@@ -138,6 +169,19 @@
*/
void scan(String tableName, Collection<Column> columns, String filterExpression, long minTime, ResultHandler handler) throws IOException;
+ /**
+ * Scans the given table using the optional filter criteria and passing each result to the provided handler.
+ *
+ * @param tableName the name of an HBase table to scan
+ * @param columns optional columns to return, if not specified all columns are returned
+ * @param filterExpression optional filter expression, if not specified no filtering is performed
+ * @param minTime the minimum timestamp of cells to return, passed to the HBase scanner timeRange
+ * @param authorizations the visibility labels to apply to the scanner.
+ * @param handler a handler to process rows of the result set
+ * @throws IOException thrown when there are communication errors with HBase
+ */
+ void scan(String tableName, Collection<Column> columns, String filterExpression, long minTime, List<String> authorizations, ResultHandler handler) throws IOException;
--- End diff --
I originally screwed up the naming here, and adopted a more Accumulo-like convention to make it easier. Authorizations are a flat list of tokens, labels are the complex boolean string that is evaluated with those authorizations during the scan.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184268871
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -336,51 +348,86 @@ public void shutdown() {
}
}
+ private static final byte[] EMPTY_VIS_STRING;
+
+ static {
+ try {
+ EMPTY_VIS_STRING = "".getBytes("UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ private List<Put> buildPuts(byte[] rowKey, List<PutColumn> columns) {
+ List<Put> retVal = new ArrayList<>();
+
+ try {
+ Put put = null;
+
+ for (final PutColumn column : columns) {
+ if (put == null || (put.getCellVisibility() == null && column.getVisibility() != null) || ( put.getCellVisibility() != null
+ && !put.getCellVisibility().getExpression().equals(column.getVisibility())
+ )) {
+ put = new Put(rowKey);
+
+ if (column.getVisibility() != null) {
+ put.setCellVisibility(new CellVisibility(column.getVisibility()));
+ }
+ retVal.add(put);
+ }
+
+ if (column.getTimestamp() != null) {
+ put.addColumn(
+ column.getColumnFamily(),
+ column.getColumnQualifier(),
+ column.getTimestamp(),
+ column.getBuffer());
+ } else {
+ put.addColumn(
+ column.getColumnFamily(),
+ column.getColumnQualifier(),
+ column.getBuffer());
+ }
+ }
+ } catch (DeserializationException de) {
+ getLogger().error("Error writing cell visibility statement.", de);
+ throw new RuntimeException(de);
+ }
+
+ return retVal;
+ }
+
@Override
public void put(final String tableName, final Collection<PutFlowFile> puts) throws IOException {
try (final Table table = connection.getTable(TableName.valueOf(tableName))) {
// Create one Put per row....
final Map<String, Put> rowPuts = new HashMap<>();
--- End diff --
This `rowPuts` is not used any longer. Please remove it.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r173484771
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -349,18 +399,54 @@ public boolean checkAndPut(final String tableName, final byte[] rowId, final byt
@Override
public void delete(final String tableName, final byte[] rowId) throws IOException {
+ delete(tableName, rowId, null);
+ }
+
+ @Override
+ public void delete(String tableName, byte[] rowId, String visibilityLabel) throws IOException {
try (final Table table = connection.getTable(TableName.valueOf(tableName))) {
Delete delete = new Delete(rowId);
+ if (visibilityLabel != null && !visibilityLabel.trim().equals("")) {
+ delete.setCellVisibility(new CellVisibility(visibilityLabel));
+ }
table.delete(delete);
}
}
@Override
public void delete(String tableName, List<byte[]> rowIds) throws IOException {
+ delete(tableName, rowIds);
+ }
+
+ @Override
+ public void deleteCells(String tableName, List<DeleteRequest> deletes) throws IOException {
+ List<Delete> deleteRequests = new ArrayList<>();
+ for (int index = 0; index < deletes.size(); index++) {
+ DeleteRequest req = deletes.get(index);
+ Delete delete = new Delete(req.getRowId())
+ .addColumn(req.getColumnFamily(), req.getColumnQualifier());
+ if (req.getVisibilityLabel() != null && !req.getVisibilityLabel().trim().equals("")) {
+ delete.setCellVisibility(new CellVisibility(req.getVisibilityLabel()));
+ }
+ deleteRequests.add(delete);
+ }
+ batchDelete(tableName, deleteRequests);
+ }
+
+ @Override
+ public void delete(String tableName, List<byte[]> rowIds, String visibilityLabel) throws IOException {
List<Delete> deletes = new ArrayList<>();
for (int index = 0; index < rowIds.size(); index++) {
- deletes.add(new Delete(rowIds.get(index)));
+ Delete delete = new Delete(rowIds.get(index));
+ if (visibilityLabel != null && !visibilityLabel.trim().equals("")) {
--- End diff --
Done.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@ijokarumawak version reverted and building...
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r182954434
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/VisibilityFetchSupport.java ---
@@ -0,0 +1,53 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.util.StringUtils;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public interface VisibilityFetchSupport {
+ PropertyDescriptor AUTHORIZATIONS = new PropertyDescriptor.Builder()
+ .name("hbase-fetch-row-authorizations")
+ .displayName("Authorizations")
+ .description("The list of authorizations to pass to the scanner. This will be ignored if cell visibility labels are not in use.")
--- End diff --
@MikeThomsen Thanks for clarifying. To understand the feature more, I have done following test using HBase shell:
```
# Created test table, and define labels
create 't1', {NAME => 'f1'}, {NAME => 'f2'}, {NAME => 'f3'}
add_labels [ 'admin', 'service', 'developer', 'test' ]
# Setup nifi user
grant 'nifi', 'RW', 't1'
set_auths 'nifi', ['test', 'developer']
# Added some cells with different Visibility Labels
put 't1', 'r1', 'f1:c1', 'value-f1:c1', {VISIBILITY=>'admin|test'}
put 't1', 'r1', 'f1:c2', 'value-f1:c2', {VISIBILITY=>'admin'}
put 't1', 'r1', 'f1:c3', 'value-f1:c3', {VISIBILITY=>'test'}
put 't1', 'r1', 'f1:c4', 'value-f1:c4', {VISIBILITY=>'developer'}
put 't1', 'r1', 'f1:c5', 'value-f1:c5', {VISIBILITY=>'test&developer'}
```
```
# == 1 ==
# Scan with 'nifi' user, without specifying any label
# 'c2' can not be read because 'nifi' is not associated with 'admin'
ROW COLUMN+CELL
r1 column=f1:c1, timestamp=1524108017481, value=value-f1:c1
r1 column=f1:c3, timestamp=1524108086777, value=value-f1:c3
r1 column=f1:c4, timestamp=1524112589432, value=value-f1:c4
r1 column=f1:c5, timestamp=1524203679856, value=value-f1:c5
1 row(s) in 0.0310 seconds
```
```
# == 2 ==
# Scan with 'nifi' and 'test' label
# 'c5' can not be read because it also needs 'developer' label.
hbase(main):016:0* scan 't1', {AUTHORIZATIONS => ['test']}
ROW COLUMN+CELL
r1 column=f1:c1, timestamp=1524108017481, value=value-f1:c1
r1 column=f1:c3, timestamp=1524108086777, value=value-f1:c3
1 row(s) in 0.0160 seconds
```
```
# == 3 ==
# Specify both 'test' and 'developer', result equals the one without any label
hbase(main):017:0> scan 't1', {AUTHORIZATIONS => ['test', 'developer']}
ROW COLUMN+CELL
r1 column=f1:c1, timestamp=1524108017481, value=value-f1:c1
r1 column=f1:c3, timestamp=1524108086777, value=value-f1:c3
r1 column=f1:c4, timestamp=1524112589432, value=value-f1:c4
r1 column=f1:c5, timestamp=1524203679856, value=value-f1:c5
1 row(s) in 0.0120 seconds
```
```
# == 4 ==
# Specifying label like this does not return anything.
hbase(main):018:0> scan 't1', {AUTHORIZATIONS => ['test&developer']}
ROW COLUMN+CELL
0 row(s) in 0.0050 seconds
```
I think what people configure at the current 'AUTHORIZATIONS' property is not an authorizations but labels. IMHO, HBase shell command parameter name does not sound natural to me.. especially for Scan and Delete. My understandings are:
- When `Put` a cell, `Visibility Label Expression` can be added, which consists of one or more Visibility Labels combined with logical operators such as '|', '&' or '!'.
- When `Scan` cells, `Visibility Labels` can be specified. Multiple labels can be specified as a comma separated string. If not specified, all of Visibility Labels associated with the user is used. Unassociated labels are dropped. This can be used to filter scan results.
- `Delete` is same as `Scan`. `Visibility Labels` can filter which cells to delete.
- With 'Get', `Visibility Labels` is not used.
My concern is, naming such parameter as 'Authorizations', it sounds as if users need to set something. Actually Authorization can be done correctly if user leaves the parameter blank.
Do you still think 'Authorizations' is the better naming??
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r182801262
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/test/java/org/apache/nifi/hbase/VisibilityIT.java ---
@@ -0,0 +1,510 @@
+/*
+ * 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.hbase;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.client.security.SecurityCapability;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.security.access.SecureTestUtil;
+import org.apache.hadoop.hbase.security.visibility.CellVisibility;
+import org.apache.hadoop.hbase.security.visibility.VisibilityClient;
+import org.apache.nifi.serialization.SimpleRecordSchema;
+import org.apache.nifi.serialization.record.MapRecord;
+import org.apache.nifi.serialization.record.MockRecordParser;
+import org.apache.nifi.serialization.record.RecordField;
+import org.apache.nifi.serialization.record.RecordFieldType;
+import org.apache.nifi.serialization.record.RecordSchema;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.security.PrivilegedExceptionAction;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+import static org.apache.hadoop.hbase.security.visibility.VisibilityConstants.LABELS_TABLE_NAME;
+
+public class VisibilityIT {
--- End diff --
I'd really like to get to a point where there are no dependencies on any real hbase modules in the processors module (even no test dependencies), that was the intent of this PR: https://github.com/apache/nifi/pull/2636
I know that having an IT test is nice to test the full scope, but is there some way this could be changed to an IT test that just tests the client service impl and could be moved into the hbase_1_1_2-client-service module?
That way all those new test dependencies would go with the actual client service implementation.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@ijokarumawak changes pushed. Ready for review.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r183014568
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/VisibilityFetchSupport.java ---
@@ -0,0 +1,53 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.util.StringUtils;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public interface VisibilityFetchSupport {
+ PropertyDescriptor AUTHORIZATIONS = new PropertyDescriptor.Builder()
+ .name("hbase-fetch-row-authorizations")
+ .displayName("Authorizations")
+ .description("The list of authorizations to pass to the scanner. This will be ignored if cell visibility labels are not in use.")
--- End diff --
> Do you still think 'Authorizations' is the better naming??
I do because we really need to draw a firm distinction between the label that is being written to the cell's metadata and the list of privileges.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184272408
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/DeleteHBaseCells.java ---
@@ -0,0 +1,139 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import org.apache.nifi.annotation.behavior.WritesAttribute;
+import org.apache.nifi.annotation.behavior.WritesAttributes;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Scanner;
+
+@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
+@WritesAttributes({
+ @WritesAttribute(attribute = "error.line", description = "The line number of the error."),
+ @WritesAttribute(attribute = "error.msg", description = "The message explaining the error.")
+})
+@Tags({"hbase", "delete", "cell", "cells", "visibility"})
+@CapabilityDescription("This processor allows the user to delete individual HBase cells by specifying one or more lines " +
+ "in the flowfile content that are a sequence composed of row ID, column family, column qualifier and associated visibility labels " +
+ "if visibility labels are enabled and in use. A user-defined separator is used to separate each of these pieces of data on each " +
+ "line, with :::: being the default separator.")
+public class DeleteHBaseCells extends AbstractDeleteHBase {
+ static final PropertyDescriptor SEPARATOR = new PropertyDescriptor.Builder()
+ .name("delete-hbase-cell-separator")
+ .displayName("Separator")
+ .description("Each line of the flowfile content is separated into components for building a delete using this" +
+ "separator. It should be something other than a single colon or a comma because these are values that " +
+ "are associated with columns and visibility labels respectively. To delete a row with ID xyz, column family abc, " +
+ "column qualifier def and visibility label PII&PHI, one would specify xyz::::abc::::def::::PII&PHI given the default " +
+ "value")
+ .required(true)
+ .defaultValue("::::")
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .build();
+
+ static final String ERROR_LINE = "error.line";
+ static final String ERROR_MSG = "error.msg";
+
+ @Override
+ protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
+ final List<PropertyDescriptor> properties = new ArrayList<>();
+ properties.add(HBASE_CLIENT_SERVICE);
+ properties.add(TABLE_NAME);
+ properties.add(SEPARATOR);
+
+ return properties;
+ }
+
+ private FlowFile writeErrorAttributes(int line, String msg, FlowFile file, ProcessSession session) {
+ file = session.putAttribute(file, ERROR_LINE, String.valueOf(line));
+ file = session.putAttribute(file, ERROR_MSG, msg != null ? msg : "");
+ return file;
+ }
+
+ private void logCell(String rowId, String family, String column, String visibility) {
+ StringBuilder sb = new StringBuilder()
+ .append("Assembling cell delete for...\t")
+ .append(String.format("Row ID: %s\t", rowId))
+ .append(String.format("Column Family: %s\t", family))
+ .append(String.format("Column Qualifier: %s\t", column))
+ .append(String.format("Visibility Label: %s", visibility));
+ getLogger().debug(sb.toString());
+ }
+
+ @Override
+ protected void doDelete(ProcessContext context, ProcessSession session) throws Exception {
+ FlowFile input = session.get();
+ if (input == null) {
+ return;
+ }
+
+ final String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(input).getValue();
+ final String tableName = context.getProperty(TABLE_NAME).evaluateAttributeExpressions(input).getValue();
+ try (InputStream is = session.read(input)) {
+ Scanner scanner = new Scanner(is);
+ int lineNum = 1;
+ List<DeleteRequest> deletes = new ArrayList<>();
+ while (scanner.hasNextLine()) {
+ String line = scanner.nextLine().trim();
+ if (line.equals("")) {
+ continue;
+ }
+ String[] parts = line.split(separator);
+ if (parts.length < 3 || parts.length > 4) {
+ final String msg = String.format("Invalid line length. It must have 3 or 4 components. It had %d.", parts.length);
+ input = writeErrorAttributes(lineNum, msg, input, session);
+ session.transfer(input, REL_FAILURE);
+ getLogger().error(msg);
+ return;
+ }
+ String rowId = parts[0];
+ String family = parts[1];
+ String column = parts[2];
+ String visibility = parts.length == 4 ? parts[3] : null;
+
+ DeleteRequest request = new DeleteRequest(rowId.getBytes(), family.getBytes(), column.getBytes(), visibility);
+ deletes.add(request);
+
+ if (getLogger().isDebugEnabled()) {
+ logCell(rowId, family, column, visibility);
+ }
+
+ lineNum++;
+ }
+ is.close();
+ clientService.deleteCells(tableName, deletes);
--- End diff --
Please add `session.getProvenanceReporter().invokeRemoteProcess(flowFile, transitUrl);` similar to what DeleteHBaseRow in order to produce NiFi provenance events.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@ijokarumawak Anything left before you're comfortable merging this?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r173485840
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/DeleteHBaseRow.java ---
@@ -103,6 +103,17 @@
.defaultValue("UTF-8")
.addValidator(StandardValidators.CHARACTER_SET_VALIDATOR)
.build();
+ static final PropertyDescriptor VISIBLITY_LABEL = new PropertyDescriptor.Builder()
+ .name("delete-visibility-label")
+ .displayName("Visibility Label")
+ .description("If visibility labels are enabled, a row cannot be deleted without supplying its visibility label(s) in the delete " +
+ "request. Note: this visibility label will be applied to all cells within the row that is specified. If some cells have " +
+ "different visibility labels, they will not be deleted. When that happens, the failure to delete will be considered a success " +
+ "because HBase does not report it as a failure.")
+ .required(false)
--- End diff --
I'm not sure about that, but you have a point on being proactive.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r172973981
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -385,7 +427,18 @@ protected PutFlowFile createPut(ProcessContext context, Record record, RecordSch
if (fieldValueBytes != null) {
- columns.add(new PutColumn(fam, clientService.toBytes(name), fieldValueBytes, timestamp));
+ PutColumn column;
+
+ String visString = (visField != null && visSettings != null && visSettings.containsKey(name))
+ ? (String)visSettings.get(name) : defaultVisibility;
+
+ if (visString != null && !visString.equals("") ) {
--- End diff --
Same here
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184272910
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/DeleteHBaseCells.java ---
@@ -0,0 +1,139 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import org.apache.nifi.annotation.behavior.WritesAttribute;
+import org.apache.nifi.annotation.behavior.WritesAttributes;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Scanner;
+
+@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
+@WritesAttributes({
+ @WritesAttribute(attribute = "error.line", description = "The line number of the error."),
+ @WritesAttribute(attribute = "error.msg", description = "The message explaining the error.")
+})
+@Tags({"hbase", "delete", "cell", "cells", "visibility"})
+@CapabilityDescription("This processor allows the user to delete individual HBase cells by specifying one or more lines " +
+ "in the flowfile content that are a sequence composed of row ID, column family, column qualifier and associated visibility labels " +
+ "if visibility labels are enabled and in use. A user-defined separator is used to separate each of these pieces of data on each " +
+ "line, with :::: being the default separator.")
+public class DeleteHBaseCells extends AbstractDeleteHBase {
--- End diff --
Do you think it would be helpful if this processor and DeleteHBaseRow support default visibility label expression as PutHBaseXX processors?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r173483514
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -385,7 +427,18 @@ protected PutFlowFile createPut(ProcessContext context, Record record, RecordSch
if (fieldValueBytes != null) {
- columns.add(new PutColumn(fam, clientService.toBytes(name), fieldValueBytes, timestamp));
+ PutColumn column;
+
+ String visString = (visField != null && visSettings != null && visSettings.containsKey(name))
+ ? (String)visSettings.get(name) : defaultVisibility;
+
+ if (visString != null && !visString.equals("") ) {
--- End diff --
Done.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184288776
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -131,6 +133,59 @@ public void onScheduled(final ProcessContext context) {
clientService = context.getProperty(HBASE_CLIENT_SERVICE).asControllerService(HBaseClientService.class);
}
+ @Override
+ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
+ if (propertyDescriptorName.startsWith("visibility.")) {
+ String[] parts = propertyDescriptorName.split("\\.");
+ String displayName;
+ String description;
+
+ if (parts.length == 2) {
+ displayName = String.format("Column Family %s Default Visibility", parts[1]);
+ description = String.format("Default visibility setting for %s", parts[1]);
+ } else if (parts.length == 3) {
+ displayName = String.format("Column Qualifier %s.%s Default Visibility", parts[1], parts[2]);
+ description = String.format("Default visibility setting for %s.%s", parts[1], parts[2]);
+ } else {
+ return null;
+ }
+
+ return new PropertyDescriptor.Builder()
+ .name(propertyDescriptorName)
+ .displayName(displayName)
+ .description(description)
+ .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+ .dynamic(true)
+ .build();
+ }
+
+ return null;
+ }
+
+ protected String pickVisibilityString(String columnFamily, String columnQualifier, FlowFile flowFile, ProcessContext context) {
+ final String lookupKey = String.format("visibility.%s.%s", columnFamily, columnQualifier);
+ final String fromAttribute = flowFile.getAttribute(lookupKey);
+ if (fromAttribute != null) {
+ return fromAttribute;
+ } else {
+ PropertyValue descriptor = context.getProperty(lookupKey);
+ if (descriptor == null) {
+ descriptor = context.getProperty(String.format("visibility.%s", columnFamily));
+ }
+
+ String retVal = descriptor != null ? descriptor.getValue() : null;
+
+ return retVal;
+ }
+ }
+
+ protected String pickVisibilityString(String defaultVisibilityString, String columnFamily, String columnQualifier, FlowFile flowFile) {
--- End diff --
This method is not used any longer. Please remove it.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/2518
I think we are saying the same thing, the authorizations can come from anywhere, but I was just using end-user as an example.
In Accumulo you would issue an operation and pass in an array of labels like [TS, S].
Cells that have (TS), (S), (TS || S), or (TS & S) would all match.
Cells that have (TS & FOO) would not.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184279789
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -142,6 +161,16 @@
.allowableValues(NULL_FIELD_EMPTY, NULL_FIELD_SKIP)
.build();
+ protected static final PropertyDescriptor VISIBILITY_RECORD_PATH = new PropertyDescriptor.Builder()
+ .name("put-hb-rec-visibility-record-path")
+ .displayName("Visibility String Record Path Root")
+ .description("A record path that points to part of the record which contains a path to a mapping of visibility strings to record paths")
+ .required(false)
+ .addValidator(Validator.VALID)
+ .build();
--- End diff --
I thought the record path is for pointing a record field containing a single visibility expression String value at the first time. But this expect the record path target to be a Map, which contains keys for specifying column qualifier to apply the visibility and value is the visibility expression.
I propose to describe that at lease. Moreover, it would be helpful if we provide an Additional details page with some input record or json, sample configurations and result in HBase cells. This feature can be useful but fairy complex to use at the first time.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@bbende that is my understanding as well.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@joshelser Unrelated to this...
Would you be open to a code review in the future (months away at least) if I were to start adding Accumulo support?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r173484741
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -349,18 +399,54 @@ public boolean checkAndPut(final String tableName, final byte[] rowId, final byt
@Override
public void delete(final String tableName, final byte[] rowId) throws IOException {
+ delete(tableName, rowId, null);
+ }
+
+ @Override
+ public void delete(String tableName, byte[] rowId, String visibilityLabel) throws IOException {
try (final Table table = connection.getTable(TableName.valueOf(tableName))) {
Delete delete = new Delete(rowId);
+ if (visibilityLabel != null && !visibilityLabel.trim().equals("")) {
+ delete.setCellVisibility(new CellVisibility(visibilityLabel));
+ }
table.delete(delete);
}
}
@Override
public void delete(String tableName, List<byte[]> rowIds) throws IOException {
+ delete(tableName, rowIds);
+ }
+
+ @Override
+ public void deleteCells(String tableName, List<DeleteRequest> deletes) throws IOException {
+ List<Delete> deleteRequests = new ArrayList<>();
+ for (int index = 0; index < deletes.size(); index++) {
+ DeleteRequest req = deletes.get(index);
+ Delete delete = new Delete(req.getRowId())
+ .addColumn(req.getColumnFamily(), req.getColumnQualifier());
+ if (req.getVisibilityLabel() != null && !req.getVisibilityLabel().trim().equals("")) {
--- End diff --
Done.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/2518
@MikeThomsen @ijokarumawak I haven't looked at how Hbase visibility labels work compared to Accumulo, but in Accumulo you pass in the authorizations for an operations which are then compared against the visibility strings on each cell. The authorizations on the operation usually come from authenticating an end-user against LDAP and then running an operation on their behalf.
When scanning cells for the current operation, if the passed in authorizations don't meet the visiblity string for a given cell, then it is as if this cell doesn't exist. So if you were issuing a delete on behalf of an end-user, I would expect they can only delete cells that are visible to them based on their authorizations.
Curious to hear that Josh has to say.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r172977733
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/DeleteHBaseRow.java ---
@@ -103,6 +103,17 @@
.defaultValue("UTF-8")
.addValidator(StandardValidators.CHARACTER_SET_VALIDATOR)
.build();
+ static final PropertyDescriptor VISIBLITY_LABEL = new PropertyDescriptor.Builder()
+ .name("delete-visibility-label")
+ .displayName("Visibility Label")
+ .description("If visibility labels are enabled, a row cannot be deleted without supplying its visibility label(s) in the delete " +
+ "request. Note: this visibility label will be applied to all cells within the row that is specified. If some cells have " +
+ "different visibility labels, they will not be deleted. When that happens, the failure to delete will be considered a success " +
+ "because HBase does not report it as a failure.")
+ .required(false)
--- End diff --
Since this is not required and always valid, a user could force the property to be set with an empty string. Is that OK? It means visibility would be equal to "". Does that make sense in some cases? Wouldn't be better to add a non-empty string validator?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184271984
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -131,6 +133,59 @@ public void onScheduled(final ProcessContext context) {
clientService = context.getProperty(HBASE_CLIENT_SERVICE).asControllerService(HBaseClientService.class);
}
+ @Override
+ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
--- End diff --
Thanks for making this to use dynamic properties it became more powerful! Would you add some docs on this functionality? You can use `@DynamicProperty` annotation to do so. Please refer HBase client service on how it is displayed in docs.
https://nifi.apache.org/docs/nifi-docs/components/org.apache.nifi/nifi-hbase_1_1_2-client-service-nar/1.6.0/org.apache.nifi.hbase.HBase_1_1_2_ClientService/index.html
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184267868
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/pom.xml ---
@@ -25,7 +25,7 @@
<artifactId>nifi-hbase_1_1_2-client-service</artifactId>
<packaging>jar</packaging>
<properties>
- <hbase.version>1.1.2</hbase.version>
+ <hbase.version>1.1.13</hbase.version>
--- End diff --
@bbende Is this gap between here and the nar name going to be an issue somewhere? Do we need to create nifi-hbase_1_1_13-client-service?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r182618909
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase-client-service-api/src/main/java/org/apache/nifi/hbase/HBaseClientService.java ---
@@ -138,6 +169,19 @@
*/
void scan(String tableName, Collection<Column> columns, String filterExpression, long minTime, ResultHandler handler) throws IOException;
+ /**
+ * Scans the given table using the optional filter criteria and passing each result to the provided handler.
+ *
+ * @param tableName the name of an HBase table to scan
+ * @param columns optional columns to return, if not specified all columns are returned
+ * @param filterExpression optional filter expression, if not specified no filtering is performed
+ * @param minTime the minimum timestamp of cells to return, passed to the HBase scanner timeRange
+ * @param authorizations the visibility labels to apply to the scanner.
+ * @param handler a handler to process rows of the result set
+ * @throws IOException thrown when there are communication errors with HBase
+ */
+ void scan(String tableName, Collection<Column> columns, String filterExpression, long minTime, List<String> authorizations, ResultHandler handler) throws IOException;
--- End diff --
I believe the argument name for the `List<String>` should be 'visibilityLabels' at this point. `org.apache.hadoop.hbase.security.visibility.Authorizations` is created later by using these labels. The same comment goes for other overloaded scan methods.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
This also adds a new processor: DeleteHBaseCells.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184737494
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -131,6 +133,59 @@ public void onScheduled(final ProcessContext context) {
clientService = context.getProperty(HBASE_CLIENT_SERVICE).asControllerService(HBaseClientService.class);
}
+ @Override
+ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
+ if (propertyDescriptorName.startsWith("visibility.")) {
+ String[] parts = propertyDescriptorName.split("\\.");
+ String displayName;
+ String description;
+
+ if (parts.length == 2) {
+ displayName = String.format("Column Family %s Default Visibility", parts[1]);
+ description = String.format("Default visibility setting for %s", parts[1]);
+ } else if (parts.length == 3) {
+ displayName = String.format("Column Qualifier %s.%s Default Visibility", parts[1], parts[2]);
+ description = String.format("Default visibility setting for %s.%s", parts[1], parts[2]);
+ } else {
+ return null;
+ }
+
+ return new PropertyDescriptor.Builder()
+ .name(propertyDescriptorName)
+ .displayName(displayName)
+ .description(description)
+ .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+ .dynamic(true)
+ .build();
+ }
+
+ return null;
+ }
+
+ protected String pickVisibilityString(String columnFamily, String columnQualifier, FlowFile flowFile, ProcessContext context) {
+ final String lookupKey = String.format("visibility.%s.%s", columnFamily, columnQualifier);
+ final String fromAttribute = flowFile.getAttribute(lookupKey);
+ if (fromAttribute != null) {
+ return fromAttribute;
+ } else {
+ PropertyValue descriptor = context.getProperty(lookupKey);
+ if (descriptor == null) {
--- End diff --
Second part added, will add unit tests in another commit.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r187529747
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/resources/META-INF/services/docs.org.apache.nifi.hbase.PutHBaseRecord/additionalDetails.html ---
@@ -0,0 +1,113 @@
+<!DOCTYPE html>
+<html lang="en">
+<!--
+ 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.
+-->
+<head>
+ <meta charset="utf-8" />
+ <title>PutHBaseRecord</title>
+ <link rel="stylesheet" href="../../../../../css/component-usage.css" type="text/css" />
+</head>
+
+<body>
+<!-- Processor Documentation ================================================== -->
+<h2>Visibility Labels:</h2>
+<p>
+ PutHBaseRecord provides the ability to define a branch of the record as a map which contains an association between
+ column qualifiers and the visibility label that they should have assigned to them.
+</p>
+<h3>Example Schema</h3>
+<pre>
+{
+ "type": "record",
+ "name": "SampleRecord",
+ "fields": [
+ { "name": "name", "type": "string" },
+ { "name": "address", "type": "string" },
+ { "name": "dob", "type": "string" },
+ { "name": "attendingPhysician", "type": "string" },
+ { "name": "accountNumber", "type": "string" },
+ { "name": "visibility_labels", "type": "map", "values": "string" }
--- End diff --
This should be
```
{ "name": "visibility_labels", "type": { "type" : "map", "values": "string" } }
```
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/2518
@MikeThomsen @ijokarumawak I'd say unless there is something in the 1.2 client that is needed to implement the visibility labels, then it might be safer to just stay on 1.1.2, and hopefully soon we can put out a new NAR for the 2.0.0 client service, and of course name it "2_x_x" or something. I wish I had named the current service and NAR "1_1_x" or "1_x_x" from the beginning :(
If we renamed things now then we'd have to rename the NAR and three services - HBase_1_1_2_ClientService, HBase_1_1_2_RecordLookupService, and HBase_1_1_2_MapCacheClientService... and then anyone who has an existing flow with those services will have ghosted components that can no longer find those class names, and then have to recreate them with the new class to get it working again.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r173455925
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseCell.java ---
@@ -96,16 +97,18 @@ protected PutFlowFile createPut(final ProcessSession session, final ProcessConte
final byte[] buffer = new byte[(int) flowFile.getSize()];
- session.read(flowFile, new InputStreamCallback() {
- @Override
- public void process(final InputStream in) throws IOException {
- StreamUtils.fillBuffer(in, buffer);
- }
- });
+ session.read(flowFile, in -> StreamUtils.fillBuffer(in, buffer));
+ PutColumn column = null;
+ if (visibilityStringToUse != null && !visibilityStringToUse.equals("")) {
--- End diff --
I like that. Done.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r173482568
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -194,6 +220,12 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
final String fieldEncodingStrategy = context.getProperty(FIELD_ENCODING_STRATEGY).getValue();
final String complexFieldStrategy = context.getProperty(COMPLEX_FIELD_STRATEGY).getValue();
final String rowEncodingStrategy = context.getProperty(ROW_ID_ENCODING_STRATEGY).getValue();
+ final String recordPathText = context.getProperty(VISIBILITY_RECORD_PATH).getValue();
+
+ RecordPath recordPath = null;
+ if (recordPathCache != null && recordPathText != null && !recordPathText.equals("")) {
--- End diff --
Done.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184734660
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/DeleteHBaseCells.java ---
@@ -0,0 +1,139 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import org.apache.nifi.annotation.behavior.WritesAttribute;
+import org.apache.nifi.annotation.behavior.WritesAttributes;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Scanner;
+
+@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
+@WritesAttributes({
+ @WritesAttribute(attribute = "error.line", description = "The line number of the error."),
+ @WritesAttribute(attribute = "error.msg", description = "The message explaining the error.")
+})
+@Tags({"hbase", "delete", "cell", "cells", "visibility"})
+@CapabilityDescription("This processor allows the user to delete individual HBase cells by specifying one or more lines " +
+ "in the flowfile content that are a sequence composed of row ID, column family, column qualifier and associated visibility labels " +
+ "if visibility labels are enabled and in use. A user-defined separator is used to separate each of these pieces of data on each " +
+ "line, with :::: being the default separator.")
+public class DeleteHBaseCells extends AbstractDeleteHBase {
+ static final PropertyDescriptor SEPARATOR = new PropertyDescriptor.Builder()
+ .name("delete-hbase-cell-separator")
+ .displayName("Separator")
+ .description("Each line of the flowfile content is separated into components for building a delete using this" +
+ "separator. It should be something other than a single colon or a comma because these are values that " +
+ "are associated with columns and visibility labels respectively. To delete a row with ID xyz, column family abc, " +
+ "column qualifier def and visibility label PII&PHI, one would specify xyz::::abc::::def::::PII&PHI given the default " +
+ "value")
+ .required(true)
+ .defaultValue("::::")
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+ .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .build();
+
+ static final String ERROR_LINE = "error.line";
+ static final String ERROR_MSG = "error.msg";
+
+ @Override
+ protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
+ final List<PropertyDescriptor> properties = new ArrayList<>();
+ properties.add(HBASE_CLIENT_SERVICE);
+ properties.add(TABLE_NAME);
+ properties.add(SEPARATOR);
+
+ return properties;
+ }
+
+ private FlowFile writeErrorAttributes(int line, String msg, FlowFile file, ProcessSession session) {
+ file = session.putAttribute(file, ERROR_LINE, String.valueOf(line));
+ file = session.putAttribute(file, ERROR_MSG, msg != null ? msg : "");
+ return file;
+ }
+
+ private void logCell(String rowId, String family, String column, String visibility) {
+ StringBuilder sb = new StringBuilder()
+ .append("Assembling cell delete for...\t")
+ .append(String.format("Row ID: %s\t", rowId))
+ .append(String.format("Column Family: %s\t", family))
+ .append(String.format("Column Qualifier: %s\t", column))
+ .append(String.format("Visibility Label: %s", visibility));
+ getLogger().debug(sb.toString());
+ }
+
+ @Override
+ protected void doDelete(ProcessContext context, ProcessSession session) throws Exception {
+ FlowFile input = session.get();
+ if (input == null) {
+ return;
+ }
+
+ final String separator = context.getProperty(SEPARATOR).evaluateAttributeExpressions(input).getValue();
+ final String tableName = context.getProperty(TABLE_NAME).evaluateAttributeExpressions(input).getValue();
+ try (InputStream is = session.read(input)) {
+ Scanner scanner = new Scanner(is);
+ int lineNum = 1;
+ List<DeleteRequest> deletes = new ArrayList<>();
+ while (scanner.hasNextLine()) {
+ String line = scanner.nextLine().trim();
+ if (line.equals("")) {
+ continue;
+ }
+ String[] parts = line.split(separator);
+ if (parts.length < 3 || parts.length > 4) {
+ final String msg = String.format("Invalid line length. It must have 3 or 4 components. It had %d.", parts.length);
+ input = writeErrorAttributes(lineNum, msg, input, session);
+ session.transfer(input, REL_FAILURE);
+ getLogger().error(msg);
+ return;
+ }
+ String rowId = parts[0];
+ String family = parts[1];
+ String column = parts[2];
+ String visibility = parts.length == 4 ? parts[3] : null;
+
+ DeleteRequest request = new DeleteRequest(rowId.getBytes(), family.getBytes(), column.getBytes(), visibility);
+ deletes.add(request);
+
+ if (getLogger().isDebugEnabled()) {
+ logCell(rowId, family, column, visibility);
+ }
+
+ lineNum++;
+ }
+ is.close();
+ clientService.deleteCells(tableName, deletes);
+
+ session.transfer(input, REL_SUCCESS);
+ } catch (Exception ex) {
+ input = writeErrorAttributes(-1, ex.getMessage(), input, session);
--- End diff --
Done.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r173482233
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseJSON.java ---
@@ -255,7 +253,16 @@ public void process(final InputStream in) throws IOException {
final byte[] colFamBytes = columnFamily.getBytes(StandardCharsets.UTF_8);
final byte[] colQualBytes = fieldName.getBytes(StandardCharsets.UTF_8);
final byte[] colValBytes = fieldValueHolder.get();
- columns.add(new PutColumn(colFamBytes, colQualBytes, colValBytes, timestamp));
+
+ final String visibilityStringToUse = pickVisibilityString(visibilityString, columnFamily, fieldName, flowFile);
+ PutColumn column;
+ if (visibilityStringToUse != null && !visibilityStringToUse.equals("")) {
--- End diff --
Done.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r178304287
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/pom.xml ---
@@ -79,6 +80,73 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-server</artifactId>
+ <version>1.3.1</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-server</artifactId>
+ <version>1.3.1</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-hdfs</artifactId>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-hdfs</artifactId>
+ <type>test-jar</type>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-common</artifactId>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-common</artifactId>
+ <version>2.9.0</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-auth</artifactId>
+ <version>2.9.0</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-hadoop-compat</artifactId>
+ <version>1.3.1</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.hbase</groupId>
+ <artifactId>hbase-hadoop2-compat</artifactId>
+ <version>1.3.1</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-hbase_1_1_2-client-service</artifactId>
+ <version>1.6.0-SNAPSHOT</version>
--- End diff --
Can you be more specific?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184266326
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -297,6 +305,8 @@ protected Connection createConnection(final ConfigurationContext context) throws
keyTab = credentialsService.getKeytab();
}
+ this.principal = principal; //Set so it is usable from getLabelsForCurrentUser
--- End diff --
This `principal` field is not used any longer? I couldn't find `getLabelsForCurrentUser`.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184731612
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/DeleteHBaseCells.java ---
@@ -0,0 +1,139 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.annotation.behavior.InputRequirement;
+import org.apache.nifi.annotation.behavior.WritesAttribute;
+import org.apache.nifi.annotation.behavior.WritesAttributes;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.processor.ProcessSession;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Scanner;
+
+@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
+@WritesAttributes({
+ @WritesAttribute(attribute = "error.line", description = "The line number of the error."),
+ @WritesAttribute(attribute = "error.msg", description = "The message explaining the error.")
+})
+@Tags({"hbase", "delete", "cell", "cells", "visibility"})
+@CapabilityDescription("This processor allows the user to delete individual HBase cells by specifying one or more lines " +
+ "in the flowfile content that are a sequence composed of row ID, column family, column qualifier and associated visibility labels " +
+ "if visibility labels are enabled and in use. A user-defined separator is used to separate each of these pieces of data on each " +
+ "line, with :::: being the default separator.")
+public class DeleteHBaseCells extends AbstractDeleteHBase {
--- End diff --
Per @joshelser's points, I don't think so. It could cause the wrong cell to be deleted.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184726415
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -297,6 +305,8 @@ protected Connection createConnection(final ConfigurationContext context) throws
keyTab = credentialsService.getKeytab();
}
+ this.principal = principal; //Set so it is usable from getLabelsForCurrentUser
--- End diff --
Done.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184733507
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseRecord.java ---
@@ -75,6 +83,17 @@
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ protected static final PropertyDescriptor DEFAULT_VISIBILITY_STRING = new PropertyDescriptor.Builder()
--- End diff --
I'm a little leery about jumping in there because PutHBaseRecord has a simplistic method of handling complex records, and we might want to change that down the road. Using `pickVisibilityString` could complicate things.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r182792431
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/VisibilityFetchSupport.java ---
@@ -0,0 +1,53 @@
+/*
+ * 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.hbase;
+
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.flowfile.FlowFile;
+import org.apache.nifi.processor.ProcessContext;
+import org.apache.nifi.util.StringUtils;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public interface VisibilityFetchSupport {
+ PropertyDescriptor AUTHORIZATIONS = new PropertyDescriptor.Builder()
+ .name("hbase-fetch-row-authorizations")
+ .displayName("Authorizations")
+ .description("The list of authorizations to pass to the scanner. This will be ignored if cell visibility labels are not in use.")
--- End diff --
In your three cases, the visibility labels would be:
* A
* A&B
* B
Passing `[B]` would eliminate the first two.
With HBase and Accumulo, you can tell the scanner "yeah, I know have set X, but use subset Y of my authorizations" because there are so many valid use cases where the scanner has to limit itself on the authorizations. A good example is that in a user-facing deployment, the scanner user will frequently have all authorizations, but be configured on the fly to do a scan with those of a particular user sitting at the keyboard setting up a scan.
Again, the distinction between "authorizations" and labels" holds because label is the requirement statement and authorizations are the tokens used to hopefully make it evaluate to `true`.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184731887
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/ScanHBase.java ---
@@ -381,6 +384,7 @@ public void onTrigger(ProcessContext context, ProcessSession session) throws Pro
limitRows,
isReversed,
columns,
+ authorizations,
--- End diff --
I don't think so, but I agree it is a nice to have. This may be something to take to the HBase mailing list before we even set up a Jira ticket.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184269182
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -336,51 +348,86 @@ public void shutdown() {
}
}
+ private static final byte[] EMPTY_VIS_STRING;
+
+ static {
+ try {
+ EMPTY_VIS_STRING = "".getBytes("UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ private List<Put> buildPuts(byte[] rowKey, List<PutColumn> columns) {
+ List<Put> retVal = new ArrayList<>();
+
+ try {
+ Put put = null;
+
+ for (final PutColumn column : columns) {
+ if (put == null || (put.getCellVisibility() == null && column.getVisibility() != null) || ( put.getCellVisibility() != null
+ && !put.getCellVisibility().getExpression().equals(column.getVisibility())
+ )) {
+ put = new Put(rowKey);
+
+ if (column.getVisibility() != null) {
+ put.setCellVisibility(new CellVisibility(column.getVisibility()));
+ }
+ retVal.add(put);
+ }
+
+ if (column.getTimestamp() != null) {
+ put.addColumn(
+ column.getColumnFamily(),
+ column.getColumnQualifier(),
+ column.getTimestamp(),
+ column.getBuffer());
+ } else {
+ put.addColumn(
+ column.getColumnFamily(),
+ column.getColumnQualifier(),
+ column.getBuffer());
+ }
+ }
+ } catch (DeserializationException de) {
+ getLogger().error("Error writing cell visibility statement.", de);
+ throw new RuntimeException(de);
+ }
+
+ return retVal;
+ }
+
@Override
public void put(final String tableName, final Collection<PutFlowFile> puts) throws IOException {
try (final Table table = connection.getTable(TableName.valueOf(tableName))) {
// Create one Put per row....
final Map<String, Put> rowPuts = new HashMap<>();
+ final Map<String, List<PutColumn>> sorted = new HashMap<>();
+ final List<Put> newPuts = new ArrayList<>();
+
for (final PutFlowFile putFlowFile : puts) {
- //this is used for the map key as a byte[] does not work as a key.
final String rowKeyString = new String(putFlowFile.getRow(), StandardCharsets.UTF_8);
- Put put = rowPuts.get(rowKeyString);
- if (put == null) {
- put = new Put(putFlowFile.getRow());
- rowPuts.put(rowKeyString, put);
+ List<PutColumn> columns = sorted.get(rowKeyString);
+ if (columns == null) {
+ columns = new ArrayList<>();
+ sorted.put(rowKeyString, columns);
}
- for (final PutColumn column : putFlowFile.getColumns()) {
- if (column.getTimestamp() != null) {
- put.addColumn(
- column.getColumnFamily(),
- column.getColumnQualifier(),
- column.getTimestamp(),
- column.getBuffer());
- } else {
- put.addColumn(
- column.getColumnFamily(),
- column.getColumnQualifier(),
- column.getBuffer());
- }
- }
+ columns.addAll(putFlowFile.getColumns());
+ }
+
+ for (final Map.Entry<String, List<PutColumn>> entry : sorted.entrySet()) {
+ newPuts.addAll(buildPuts(entry.getKey().getBytes(StandardCharsets.UTF_8), entry.getValue()));
}
- table.put(new ArrayList<>(rowPuts.values()));
+ table.put(new ArrayList<>(newPuts)); /*rowPuts.values()));*/
--- End diff --
Do we have to wrap it with new ArrayList here? If not, lets just pass `newPuts` to avoid unnecessary object creation.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by anoopsjohn <gi...@git.apache.org>.
Github user anoopsjohn commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r187594182
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -336,51 +346,85 @@ public void shutdown() {
}
}
+ private static final byte[] EMPTY_VIS_STRING;
+
+ static {
+ try {
+ EMPTY_VIS_STRING = "".getBytes("UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ private List<Put> buildPuts(byte[] rowKey, List<PutColumn> columns) {
+ List<Put> retVal = new ArrayList<>();
+
+ try {
+ Put put = null;
+
+ for (final PutColumn column : columns) {
--- End diff --
In the list of PutColumn I can have 1st column having 'x' as visibility and next one with 'y' and 3rd one again with 'x'. So here you will create 3 Put objects I guess. Would be better if we map 'columns' into a visibility vs columns data structure and then for each of visibility create one Put (?)
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184727209
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -336,51 +348,86 @@ public void shutdown() {
}
}
+ private static final byte[] EMPTY_VIS_STRING;
+
+ static {
+ try {
+ EMPTY_VIS_STRING = "".getBytes("UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ private List<Put> buildPuts(byte[] rowKey, List<PutColumn> columns) {
+ List<Put> retVal = new ArrayList<>();
+
+ try {
+ Put put = null;
+
+ for (final PutColumn column : columns) {
+ if (put == null || (put.getCellVisibility() == null && column.getVisibility() != null) || ( put.getCellVisibility() != null
+ && !put.getCellVisibility().getExpression().equals(column.getVisibility())
+ )) {
+ put = new Put(rowKey);
+
+ if (column.getVisibility() != null) {
+ put.setCellVisibility(new CellVisibility(column.getVisibility()));
+ }
+ retVal.add(put);
+ }
+
+ if (column.getTimestamp() != null) {
+ put.addColumn(
+ column.getColumnFamily(),
+ column.getColumnQualifier(),
+ column.getTimestamp(),
+ column.getBuffer());
+ } else {
+ put.addColumn(
+ column.getColumnFamily(),
+ column.getColumnQualifier(),
+ column.getBuffer());
+ }
+ }
+ } catch (DeserializationException de) {
+ getLogger().error("Error writing cell visibility statement.", de);
+ throw new RuntimeException(de);
+ }
+
+ return retVal;
+ }
+
@Override
public void put(final String tableName, final Collection<PutFlowFile> puts) throws IOException {
try (final Table table = connection.getTable(TableName.valueOf(tableName))) {
// Create one Put per row....
final Map<String, Put> rowPuts = new HashMap<>();
--- End diff --
Done.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r188193956
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -336,51 +346,85 @@ public void shutdown() {
}
}
+ private static final byte[] EMPTY_VIS_STRING;
+
+ static {
+ try {
+ EMPTY_VIS_STRING = "".getBytes("UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ private List<Put> buildPuts(byte[] rowKey, List<PutColumn> columns) {
+ List<Put> retVal = new ArrayList<>();
+
+ try {
+ Put put = null;
+
+ for (final PutColumn column : columns) {
--- End diff --
@anoopsjohn Good point. I think you're correct. Bundling columns into the same put would be able to reduce the number of Put objects. I wonder how much performance gain we can get by doing so. If you are interested in making such optimization, please go ahead and create another PR. Before/after benchmarking result would be appreciated. Thanks!
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184275993
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -131,6 +133,59 @@ public void onScheduled(final ProcessContext context) {
clientService = context.getProperty(HBASE_CLIENT_SERVICE).asControllerService(HBaseClientService.class);
}
+ @Override
+ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
+ if (propertyDescriptorName.startsWith("visibility.")) {
+ String[] parts = propertyDescriptorName.split("\\.");
+ String displayName;
+ String description;
+
+ if (parts.length == 2) {
+ displayName = String.format("Column Family %s Default Visibility", parts[1]);
+ description = String.format("Default visibility setting for %s", parts[1]);
+ } else if (parts.length == 3) {
+ displayName = String.format("Column Qualifier %s.%s Default Visibility", parts[1], parts[2]);
+ description = String.format("Default visibility setting for %s.%s", parts[1], parts[2]);
+ } else {
+ return null;
+ }
+
+ return new PropertyDescriptor.Builder()
+ .name(propertyDescriptorName)
+ .displayName(displayName)
+ .description(description)
+ .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+ .dynamic(true)
+ .build();
+ }
+
+ return null;
+ }
+
+ protected String pickVisibilityString(String columnFamily, String columnQualifier, FlowFile flowFile, ProcessContext context) {
+ final String lookupKey = String.format("visibility.%s.%s", columnFamily, columnQualifier);
--- End diff --
`visibility.%s.%s` will be `visibility.f1.` if column family if `f1` and columnQualifier is null. I think it should be `visibility.f1` without the trailing dot in that case. How do you think?
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r184285304
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -131,6 +133,59 @@ public void onScheduled(final ProcessContext context) {
clientService = context.getProperty(HBASE_CLIENT_SERVICE).asControllerService(HBaseClientService.class);
}
+ @Override
+ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
+ if (propertyDescriptorName.startsWith("visibility.")) {
+ String[] parts = propertyDescriptorName.split("\\.");
+ String displayName;
+ String description;
+
+ if (parts.length == 2) {
+ displayName = String.format("Column Family %s Default Visibility", parts[1]);
+ description = String.format("Default visibility setting for %s", parts[1]);
+ } else if (parts.length == 3) {
+ displayName = String.format("Column Qualifier %s.%s Default Visibility", parts[1], parts[2]);
+ description = String.format("Default visibility setting for %s.%s", parts[1], parts[2]);
+ } else {
+ return null;
+ }
+
+ return new PropertyDescriptor.Builder()
+ .name(propertyDescriptorName)
+ .displayName(displayName)
+ .description(description)
+ .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+ .dynamic(true)
+ .build();
--- End diff --
These properties should be able to support EL from incoming FlowFiles. In case if user would like to define default visibility based on FlowFile content type ... etc
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@ijokarumawak New build pushed. Sorry about the squash commit wiping out the existing comments, but I had to do it in order to save time and stress on integrating the EL changes and things like that.
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
@ijokarumawak Disregard that last comment. I apparently forgot to update the EL support. Will get that done today.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r172972637
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseCell.java ---
@@ -96,16 +97,18 @@ protected PutFlowFile createPut(final ProcessSession session, final ProcessConte
final byte[] buffer = new byte[(int) flowFile.getSize()];
- session.read(flowFile, new InputStreamCallback() {
- @Override
- public void process(final InputStream in) throws IOException {
- StreamUtils.fillBuffer(in, buffer);
- }
- });
+ session.read(flowFile, in -> StreamUtils.fillBuffer(in, buffer));
+ PutColumn column = null;
+ if (visibilityStringToUse != null && !visibilityStringToUse.equals("")) {
--- End diff --
Cosmetic comment - feel free to ignore.
````java
PutColumn column = StringUtils.isEmpty(visibilityStringToUse)
? new PutColumn(columnFamily.getBytes(StandardCharsets.UTF_8),
columnQualifier.getBytes(StandardCharsets.UTF_8), buffer, timestamp)
: new PutColumn(columnFamily.getBytes(StandardCharsets.UTF_8),
columnQualifier.getBytes(StandardCharsets.UTF_8), buffer, timestamp, visibilityStringToUse);
````
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by MikeThomsen <gi...@git.apache.org>.
Github user MikeThomsen commented on the issue:
https://github.com/apache/nifi/pull/2518
Thanks for jumping on this @ijokarumawak I'll try to find some time today to resolve and push a fix.
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r172976240
--- Diff: nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java ---
@@ -349,18 +399,54 @@ public boolean checkAndPut(final String tableName, final byte[] rowId, final byt
@Override
public void delete(final String tableName, final byte[] rowId) throws IOException {
+ delete(tableName, rowId, null);
+ }
+
+ @Override
+ public void delete(String tableName, byte[] rowId, String visibilityLabel) throws IOException {
try (final Table table = connection.getTable(TableName.valueOf(tableName))) {
Delete delete = new Delete(rowId);
+ if (visibilityLabel != null && !visibilityLabel.trim().equals("")) {
+ delete.setCellVisibility(new CellVisibility(visibilityLabel));
+ }
table.delete(delete);
}
}
@Override
public void delete(String tableName, List<byte[]> rowIds) throws IOException {
+ delete(tableName, rowIds);
+ }
+
+ @Override
+ public void deleteCells(String tableName, List<DeleteRequest> deletes) throws IOException {
+ List<Delete> deleteRequests = new ArrayList<>();
+ for (int index = 0; index < deletes.size(); index++) {
+ DeleteRequest req = deletes.get(index);
+ Delete delete = new Delete(req.getRowId())
+ .addColumn(req.getColumnFamily(), req.getColumnQualifier());
+ if (req.getVisibilityLabel() != null && !req.getVisibilityLabel().trim().equals("")) {
+ delete.setCellVisibility(new CellVisibility(req.getVisibilityLabel()));
+ }
+ deleteRequests.add(delete);
+ }
+ batchDelete(tableName, deleteRequests);
+ }
+
+ @Override
+ public void delete(String tableName, List<byte[]> rowIds, String visibilityLabel) throws IOException {
List<Delete> deletes = new ArrayList<>();
for (int index = 0; index < rowIds.size(); index++) {
- deletes.add(new Delete(rowIds.get(index)));
+ Delete delete = new Delete(rowIds.get(index));
+ if (visibilityLabel != null && !visibilityLabel.trim().equals("")) {
--- End diff --
same here
---
[GitHub] nifi pull request #2518: NIFI-4637 Added support for visibility labels to th...
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2518#discussion_r172973274
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseJSON.java ---
@@ -255,7 +253,16 @@ public void process(final InputStream in) throws IOException {
final byte[] colFamBytes = columnFamily.getBytes(StandardCharsets.UTF_8);
final byte[] colQualBytes = fieldName.getBytes(StandardCharsets.UTF_8);
final byte[] colValBytes = fieldValueHolder.get();
- columns.add(new PutColumn(colFamBytes, colQualBytes, colValBytes, timestamp));
+
+ final String visibilityStringToUse = pickVisibilityString(visibilityString, columnFamily, fieldName, flowFile);
+ PutColumn column;
+ if (visibilityStringToUse != null && !visibilityStringToUse.equals("")) {
--- End diff --
Same here
---
[GitHub] nifi issue #2518: NIFI-4637 Added support for visibility labels to the HBase...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/nifi/pull/2518
> Would you please educate me why delete operation uses expression instead of a comma separated labels as scan does
The general reasoning is this: you may have multiple "values" for the same key at different levels of visibility. Consider credit-card information:
```
josh-creditcard1 f:number [private] -> 123456-1234-12345678
josh-creditcard1 f:number [seller] -> xxxxxx-xxxx-xxxx5678
```
The visibility label for Accumulo is an expression that defines if a user with a collection of visibility labels is allowed to see that record. The expression also contributes to the uniqueness of that key, almost acting as a kind of "attribute" for the record being store.
Requiring the exact visibility label to delete the record is also important in a multi-tenant system with various levels of visibility because you may not know if other copies of the Key exist that you are unaware of. For example, if a "seller" was trying to delete my creditcard1 information, they would be unaware that my full creditcard number also exists there -- if we gave acknowledgment that it was deleted, that would be an information leak.
---