You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by aperepel <gi...@git.apache.org> on 2017/06/02 20:03:41 UTC

[GitHub] nifi pull request #1886: NIFI-4005 Add support for Azure Shared Access Signa...

GitHub user aperepel opened a pull request:

    https://github.com/apache/nifi/pull/1886

    NIFI-4005 Add support for Azure Shared Access Signature (SAS) Tokens

    - Refactoring and cleanup
    - Upgraded the client library from 5.0.0 to 5.2.0

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/aperepel/nifi NIFI-4005

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi/pull/1886.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 #1886
    
----
commit 0ae8b2ffdf169724850a583668d3702af5516345
Author: Andrew Grande <ap...@gmail.com>
Date:   2017-05-31T20:45:26Z

    NIFI-4005 Add support for Azure Shared Access Signature (SAS) Tokens. Upgraded the client library from 5.0.0 to 5.2.0

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #1886: NIFI-4005 Add support for Azure Shared Access Signature (S...

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:

    https://github.com/apache/nifi/pull/1886
  
    @aperepel I took over updating the PR. The new one #2353 is submitted based on your commit. Would you close this PR? Thanks!


---

[GitHub] nifi pull request #1886: NIFI-4005 Add support for Azure Shared Access Signa...

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1886#discussion_r157141861
  
    --- Diff: nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/Azure.java ---
    @@ -0,0 +1,124 @@
    +/*
    + * 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.processors.azure.storage.utils;
    +
    +import com.microsoft.azure.storage.CloudStorageAccount;
    +import com.microsoft.azure.storage.StorageCredentials;
    +import com.microsoft.azure.storage.StorageCredentialsSharedAccessSignature;
    +import com.microsoft.azure.storage.blob.CloudBlobClient;
    +import org.apache.commons.lang3.StringUtils;
    +import org.apache.nifi.components.PropertyDescriptor;
    +import org.apache.nifi.components.ValidationContext;
    +import org.apache.nifi.components.ValidationResult;
    +import org.apache.nifi.logging.ComponentLog;
    +import org.apache.nifi.processor.ProcessContext;
    +import org.apache.nifi.processor.util.StandardValidators;
    +
    +import java.net.URI;
    +import java.net.URISyntaxException;
    +import java.security.InvalidKeyException;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.List;
    +
    +public final class Azure {
    --- End diff --
    
    The class name may be too broad IMHO. Probably `AzureStorageUtils` if it envisions to cover other storage services such as File, Queue or Table. Or `AzureBlobUtils` to be more specific. Thoughts?


---

[GitHub] nifi pull request #1886: NIFI-4005 Add support for Azure Shared Access Signa...

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1886#discussion_r157142645
  
    --- Diff: nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/Azure.java ---
    @@ -0,0 +1,124 @@
    +/*
    + * 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.processors.azure.storage.utils;
    +
    +import com.microsoft.azure.storage.CloudStorageAccount;
    +import com.microsoft.azure.storage.StorageCredentials;
    +import com.microsoft.azure.storage.StorageCredentialsSharedAccessSignature;
    +import com.microsoft.azure.storage.blob.CloudBlobClient;
    +import org.apache.commons.lang3.StringUtils;
    +import org.apache.nifi.components.PropertyDescriptor;
    +import org.apache.nifi.components.ValidationContext;
    +import org.apache.nifi.components.ValidationResult;
    +import org.apache.nifi.logging.ComponentLog;
    +import org.apache.nifi.processor.ProcessContext;
    +import org.apache.nifi.processor.util.StandardValidators;
    +
    +import java.net.URI;
    +import java.net.URISyntaxException;
    +import java.security.InvalidKeyException;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.List;
    +
    +public final class Azure {
    +    public static final String BLOCK = "Block";
    +    public static final String PAGE = "Page";
    +
    +    public static final PropertyDescriptor ACCOUNT_KEY = new PropertyDescriptor.Builder().name("storage-account-key").displayName("Storage Account Key")
    +            .description("The storage account key. This is an admin-like password providing access to every container in this account. It is recommended " +
    +                    "one uses Shared Access Signature (SAS) token instead for fine-grained control with policies. " +
    +                    "There are certain risks in allowing the account key to be stored as a flowfile" +
    +                    "attribute. While it does provide for a more flexible flow by allowing the account key to " +
    +                    "be fetched dynamically from a flow file attribute, care must be taken to restrict access to " +
    +                    "the event provenance data (e.g. by strictly controlling the policies governing provenance for this Processor). " +
    +                    "In addition, the provenance repositories may be put on encrypted disk partitions.")
    +            .addValidator(StandardValidators.NON_EMPTY_VALIDATOR).expressionLanguageSupported(true).required(false).sensitive(true).build();
    +
    +    public static final PropertyDescriptor ACCOUNT_NAME = new PropertyDescriptor.Builder().name("storage-account-name").displayName("Storage Account Name")
    +            .description("The storage account name.  There are certain risks in allowing the account name to be stored as a flowfile" +
    +                    "attribute. While it does provide for a more flexible flow by allowing the account name to " +
    +                    "be fetched dynamically from a flow file attribute, care must be taken to restrict access to " +
    +                    "the event provenance data (e.g. by strictly controlling the policies governing provenance for this Processor). " +
    +                    "In addition, the provenance repositories may be put on encrypted disk partitions.")
    +            .addValidator(StandardValidators.NON_EMPTY_VALIDATOR).expressionLanguageSupported(true).required(true).sensitive(true).build();
    +
    +    public static final PropertyDescriptor CONTAINER = new PropertyDescriptor.Builder().name("container-name").displayName("Container Name")
    +            .description("Name of the Azure storage container").addValidator(StandardValidators.NON_EMPTY_VALIDATOR).expressionLanguageSupported(true).required(true).build();
    +
    +    public static final PropertyDescriptor PROP_SAS_TOKEN = new PropertyDescriptor.Builder()
    +            .name("SAS String")
    --- End diff --
    
    Please use name and displayName as other properties do. "SAS Token" would be better go along with Azure terminology. Especially when I copied value from Azure Web console, I could be more confident where should I copy the value from if it's labeled as "SAS Token".
    ![image](https://user-images.githubusercontent.com/1107620/34032823-07539566-e1bb-11e7-95c9-ae8264e723ad.png)



---

[GitHub] nifi pull request #1886: NIFI-4005 Add support for Azure Shared Access Signa...

Posted by aperepel <gi...@git.apache.org>.
Github user aperepel closed the pull request at:

    https://github.com/apache/nifi/pull/1886


---

[GitHub] nifi issue #1886: NIFI-4005 Add support for Azure Shared Access Signature (S...

Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck commented on the issue:

    https://github.com/apache/nifi/pull/1886
  
    Reviewing...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #1886: NIFI-4005 Add support for Azure Shared Access Signa...

Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/1886#discussion_r157141180
  
    --- Diff: nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/Azure.java ---
    @@ -0,0 +1,124 @@
    +/*
    + * 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.processors.azure.storage.utils;
    +
    +import com.microsoft.azure.storage.CloudStorageAccount;
    +import com.microsoft.azure.storage.StorageCredentials;
    +import com.microsoft.azure.storage.StorageCredentialsSharedAccessSignature;
    +import com.microsoft.azure.storage.blob.CloudBlobClient;
    +import org.apache.commons.lang3.StringUtils;
    +import org.apache.nifi.components.PropertyDescriptor;
    +import org.apache.nifi.components.ValidationContext;
    +import org.apache.nifi.components.ValidationResult;
    +import org.apache.nifi.logging.ComponentLog;
    +import org.apache.nifi.processor.ProcessContext;
    +import org.apache.nifi.processor.util.StandardValidators;
    +
    +import java.net.URI;
    +import java.net.URISyntaxException;
    +import java.security.InvalidKeyException;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.List;
    +
    +public final class Azure {
    +    public static final String BLOCK = "Block";
    +    public static final String PAGE = "Page";
    +
    +    public static final PropertyDescriptor ACCOUNT_KEY = new PropertyDescriptor.Builder().name("storage-account-key").displayName("Storage Account Key")
    +            .description("The storage account key. This is an admin-like password providing access to every container in this account. It is recommended " +
    +                    "one uses Shared Access Signature (SAS) token instead for fine-grained control with policies. " +
    +                    "There are certain risks in allowing the account key to be stored as a flowfile" +
    +                    "attribute. While it does provide for a more flexible flow by allowing the account key to " +
    +                    "be fetched dynamically from a flow file attribute, care must be taken to restrict access to " +
    +                    "the event provenance data (e.g. by strictly controlling the policies governing provenance for this Processor). " +
    +                    "In addition, the provenance repositories may be put on encrypted disk partitions.")
    +            .addValidator(StandardValidators.NON_EMPTY_VALIDATOR).expressionLanguageSupported(true).required(false).sensitive(true).build();
    +
    +    public static final PropertyDescriptor ACCOUNT_NAME = new PropertyDescriptor.Builder().name("storage-account-name").displayName("Storage Account Name")
    +            .description("The storage account name.  There are certain risks in allowing the account name to be stored as a flowfile" +
    --- End diff --
    
    A whitespace is needed after the 'as a flowfile'. It's displayed as 'as a flowfileattribute.'


---

[GitHub] nifi issue #1886: NIFI-4005 Add support for Azure Shared Access Signature (S...

Posted by aperepel <gi...@git.apache.org>.
Github user aperepel commented on the issue:

    https://github.com/apache/nifi/pull/1886
  
    Thank you, Koji!


---