-
Notifications
You must be signed in to change notification settings - Fork 2.9k
NIFI-8258: Add support for Service Principal authentication in ADLS p… #4843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -26,6 +26,7 @@ | |||||
| import org.apache.nifi.components.ValidationResult; | ||||||
| import org.apache.nifi.controller.AbstractControllerService; | ||||||
| import org.apache.nifi.controller.ConfigurationContext; | ||||||
| import org.apache.nifi.expression.ExpressionLanguageScope; | ||||||
| import org.apache.nifi.processor.util.StandardValidators; | ||||||
| import org.apache.nifi.processors.azure.storage.utils.AzureStorageUtils; | ||||||
|
|
||||||
|
|
@@ -35,7 +36,6 @@ | |||||
| import java.util.Collections; | ||||||
| import java.util.List; | ||||||
| import java.util.Map; | ||||||
| import java.util.StringJoiner; | ||||||
| import java.util.function.BiConsumer; | ||||||
| import java.util.function.Function; | ||||||
|
|
||||||
|
|
@@ -49,37 +49,81 @@ | |||||
| public class ADLSCredentialsControllerService extends AbstractControllerService implements ADLSCredentialsService { | ||||||
|
|
||||||
| public static final PropertyDescriptor ACCOUNT_NAME = new PropertyDescriptor.Builder() | ||||||
| .fromPropertyDescriptor(AzureStorageUtils.ACCOUNT_NAME) | ||||||
| .description(AzureStorageUtils.ACCOUNT_NAME_BASE_DESCRIPTION) | ||||||
| .required(true) | ||||||
| .build(); | ||||||
| .fromPropertyDescriptor(AzureStorageUtils.ACCOUNT_NAME) | ||||||
| .description(AzureStorageUtils.ACCOUNT_NAME_BASE_DESCRIPTION) | ||||||
| .required(true) | ||||||
| .expressionLanguageSupported(ExpressionLanguageScope.NONE) | ||||||
| .build(); | ||||||
|
|
||||||
| public static final PropertyDescriptor ENDPOINT_SUFFIX = new PropertyDescriptor.Builder() | ||||||
| .fromPropertyDescriptor(AzureStorageUtils.ENDPOINT_SUFFIX) | ||||||
| .displayName("Endpoint Suffix") | ||||||
| .description( | ||||||
| "Storage accounts in public Azure always use a common FQDN suffix. " + | ||||||
| "Override this endpoint suffix with a different suffix in certain circumstances (like Azure Stack or non-public Azure regions).") | ||||||
| .required(true) | ||||||
| .defaultValue("dfs.core.windows.net") | ||||||
| .build(); | ||||||
| .fromPropertyDescriptor(AzureStorageUtils.ENDPOINT_SUFFIX) | ||||||
| .displayName("Endpoint Suffix") | ||||||
| .description("Storage accounts in public Azure always use a common FQDN suffix. " + | ||||||
| "Override this endpoint suffix with a different suffix in certain circumstances (like Azure Stack or non-public Azure regions).") | ||||||
| .required(true) | ||||||
| .defaultValue("dfs.core.windows.net") | ||||||
| .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY) | ||||||
| .build(); | ||||||
|
|
||||||
| public static final PropertyDescriptor ACCOUNT_KEY = new PropertyDescriptor.Builder() | ||||||
| .fromPropertyDescriptor(AzureStorageUtils.ACCOUNT_KEY) | ||||||
| .expressionLanguageSupported(ExpressionLanguageScope.NONE) | ||||||
| .build(); | ||||||
|
|
||||||
| public static final PropertyDescriptor SAS_TOKEN = new PropertyDescriptor.Builder() | ||||||
| .fromPropertyDescriptor(AzureStorageUtils.PROP_SAS_TOKEN) | ||||||
| .expressionLanguageSupported(ExpressionLanguageScope.NONE) | ||||||
| .build(); | ||||||
|
|
||||||
| public static final PropertyDescriptor USE_MANAGED_IDENTITY = new PropertyDescriptor.Builder() | ||||||
| .name("storage-use-managed-identity") | ||||||
| .displayName("Use Azure Managed Identity") | ||||||
| .description("Choose whether or not to use the managed identity of Azure VM/VMSS ") | ||||||
| .required(false) | ||||||
| .defaultValue("false") | ||||||
| .allowableValues("true", "false") | ||||||
| .addValidator(StandardValidators.BOOLEAN_VALIDATOR) | ||||||
| .build(); | ||||||
| .name("storage-use-managed-identity") | ||||||
| .displayName("Use Azure Managed Identity") | ||||||
| .description("Choose whether or not to use the managed identity of Azure VM/VMSS ") | ||||||
| .required(false) | ||||||
| .defaultValue("false") | ||||||
| .allowableValues("true", "false") | ||||||
| .addValidator(StandardValidators.BOOLEAN_VALIDATOR) | ||||||
| .build(); | ||||||
|
|
||||||
| public static final PropertyDescriptor SERVICE_PRINCIPAL_TENANT_ID = new PropertyDescriptor.Builder() | ||||||
| .name("service-principal-tenant-id") | ||||||
| .displayName("Service Principal Tenant ID") | ||||||
| .description("Tenant ID of the Azure Active Directory hosting the Service Principal. The property is required when Service Principal authentication is used.") | ||||||
| .sensitive(true) | ||||||
| .required(false) | ||||||
| .addValidator(StandardValidators.NON_BLANK_VALIDATOR) | ||||||
| .expressionLanguageSupported(ExpressionLanguageScope.NONE) | ||||||
| .build(); | ||||||
|
|
||||||
| public static final PropertyDescriptor SERVICE_PRINCIPAL_CLIENT_ID = new PropertyDescriptor.Builder() | ||||||
| .name("service-principal-client-id") | ||||||
| .displayName("Service Principal Client ID") | ||||||
| .description("Client ID (or Application ID) of the Client/Application having the Service Principal. The property is required when Service Principal authentication is used.") | ||||||
| .sensitive(true) | ||||||
| .required(false) | ||||||
| .addValidator(StandardValidators.NON_BLANK_VALIDATOR) | ||||||
| .expressionLanguageSupported(ExpressionLanguageScope.NONE) | ||||||
| .build(); | ||||||
|
|
||||||
| public static final PropertyDescriptor SERVICE_PRINCIPAL_CLIENT_SECRET = new PropertyDescriptor.Builder() | ||||||
| .name("service-principal-client-secret") | ||||||
| .displayName("Service Principal Client Secret") | ||||||
| .description("Password of the Client/Application. The property is required when Service Principal authentication is used.") | ||||||
| .sensitive(true) | ||||||
| .required(false) | ||||||
| .addValidator(StandardValidators.NON_BLANK_VALIDATOR) | ||||||
| .expressionLanguageSupported(ExpressionLanguageScope.NONE) | ||||||
| .build(); | ||||||
|
|
||||||
| private static final List<PropertyDescriptor> PROPERTIES = Collections.unmodifiableList(Arrays.asList( | ||||||
| ACCOUNT_NAME, | ||||||
| ENDPOINT_SUFFIX, | ||||||
| AzureStorageUtils.ACCOUNT_KEY, | ||||||
| AzureStorageUtils.PROP_SAS_TOKEN, | ||||||
| USE_MANAGED_IDENTITY | ||||||
| ACCOUNT_NAME, | ||||||
| ENDPOINT_SUFFIX, | ||||||
| ACCOUNT_KEY, | ||||||
| SAS_TOKEN, | ||||||
| USE_MANAGED_IDENTITY, | ||||||
| SERVICE_PRINCIPAL_TENANT_ID, | ||||||
| SERVICE_PRINCIPAL_CLIENT_ID, | ||||||
| SERVICE_PRINCIPAL_CLIENT_SECRET | ||||||
| )); | ||||||
|
|
||||||
| private ConfigurationContext context; | ||||||
|
|
@@ -93,20 +137,41 @@ protected List<PropertyDescriptor> getSupportedPropertyDescriptors() { | |||||
| protected Collection<ValidationResult> customValidate(ValidationContext validationContext) { | ||||||
| final List<ValidationResult> results = new ArrayList<>(); | ||||||
|
|
||||||
| boolean accountKeySet = StringUtils.isNotBlank(validationContext.getProperty(AzureStorageUtils.ACCOUNT_KEY).getValue()); | ||||||
| boolean sasTokenSet = StringUtils.isNotBlank(validationContext.getProperty(AzureStorageUtils.PROP_SAS_TOKEN).getValue()); | ||||||
| boolean accountKeySet = StringUtils.isNotBlank(validationContext.getProperty(ACCOUNT_KEY).getValue()); | ||||||
| boolean sasTokenSet = StringUtils.isNotBlank(validationContext.getProperty(SAS_TOKEN).getValue()); | ||||||
| boolean useManagedIdentitySet = validationContext.getProperty(USE_MANAGED_IDENTITY).asBoolean(); | ||||||
|
|
||||||
| if (!onlyOneSet(accountKeySet, sasTokenSet, useManagedIdentitySet)) { | ||||||
| StringJoiner options = new StringJoiner(", ") | ||||||
| .add(AzureStorageUtils.ACCOUNT_KEY.getDisplayName()) | ||||||
| .add(AzureStorageUtils.PROP_SAS_TOKEN.getDisplayName()) | ||||||
| .add(USE_MANAGED_IDENTITY.getDisplayName()); | ||||||
| boolean servicePrincipalTenantIdSet = StringUtils.isNotBlank(validationContext.getProperty(SERVICE_PRINCIPAL_TENANT_ID).getValue()); | ||||||
| boolean servicePrincipalClientIdSet = StringUtils.isNotBlank(validationContext.getProperty(SERVICE_PRINCIPAL_CLIENT_ID).getValue()); | ||||||
| boolean servicePrincipalClientSecretSet = StringUtils.isNotBlank(validationContext.getProperty(SERVICE_PRINCIPAL_CLIENT_SECRET).getValue()); | ||||||
|
|
||||||
| boolean servicePrincipalSet = servicePrincipalTenantIdSet || servicePrincipalClientIdSet || servicePrincipalClientSecretSet; | ||||||
|
|
||||||
| if (!onlyOneSet(accountKeySet, sasTokenSet, useManagedIdentitySet, servicePrincipalSet)) { | ||||||
| results.add(new ValidationResult.Builder().subject(this.getClass().getSimpleName()) | ||||||
| .valid(false) | ||||||
| .explanation("one and only one of [" + options + "] should be set") | ||||||
| .explanation("one and only one authentication method of [Account Key, SAS Token, Managed Identity, Service Principal] should be used") | ||||||
| .build()); | ||||||
| } else if (servicePrincipalSet) { | ||||||
| String template = "'%s' must be set when Service Principal authentication is being configured"; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just for this, I would not add a new commit. Will fix it in the follow-up jira (where |
||||||
| if (!servicePrincipalTenantIdSet) { | ||||||
| results.add(new ValidationResult.Builder().subject(this.getClass().getSimpleName()) | ||||||
| .valid(false) | ||||||
| .explanation(String.format(template, SERVICE_PRINCIPAL_TENANT_ID.getDisplayName())) | ||||||
| .build()); | ||||||
| } | ||||||
| if (!servicePrincipalClientIdSet) { | ||||||
| results.add(new ValidationResult.Builder().subject(this.getClass().getSimpleName()) | ||||||
| .valid(false) | ||||||
| .explanation(String.format(template, SERVICE_PRINCIPAL_CLIENT_ID.getDisplayName())) | ||||||
| .build()); | ||||||
| } | ||||||
| if (!servicePrincipalClientSecretSet) { | ||||||
| results.add(new ValidationResult.Builder().subject(this.getClass().getSimpleName()) | ||||||
| .valid(false) | ||||||
| .explanation(String.format(template, SERVICE_PRINCIPAL_CLIENT_SECRET.getDisplayName())) | ||||||
| .build()); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| return results; | ||||||
|
|
@@ -129,23 +194,29 @@ public void onEnabled(ConfigurationContext context) { | |||||
| public ADLSCredentialsDetails getCredentialsDetails(Map<String, String> attributes) { | ||||||
| ADLSCredentialsDetails.Builder credentialsBuilder = ADLSCredentialsDetails.Builder.newBuilder(); | ||||||
|
|
||||||
| setValue(credentialsBuilder, ACCOUNT_NAME, PropertyValue::getValue, ADLSCredentialsDetails.Builder::setAccountName); | ||||||
| setValue(credentialsBuilder, AzureStorageUtils.ACCOUNT_KEY, PropertyValue::getValue, ADLSCredentialsDetails.Builder::setAccountKey); | ||||||
| setValue(credentialsBuilder, AzureStorageUtils.PROP_SAS_TOKEN, PropertyValue::getValue, ADLSCredentialsDetails.Builder::setSasToken); | ||||||
| setValue(credentialsBuilder, ENDPOINT_SUFFIX, PropertyValue::getValue, ADLSCredentialsDetails.Builder::setEndpointSuffix); | ||||||
| setValue(credentialsBuilder, USE_MANAGED_IDENTITY, PropertyValue::asBoolean, ADLSCredentialsDetails.Builder::setUseManagedIdentity); | ||||||
| setValue(credentialsBuilder, ACCOUNT_NAME, PropertyValue::getValue, ADLSCredentialsDetails.Builder::setAccountName, attributes); | ||||||
| setValue(credentialsBuilder, ACCOUNT_KEY, PropertyValue::getValue, ADLSCredentialsDetails.Builder::setAccountKey, attributes); | ||||||
| setValue(credentialsBuilder, SAS_TOKEN, PropertyValue::getValue, ADLSCredentialsDetails.Builder::setSasToken, attributes); | ||||||
| setValue(credentialsBuilder, ENDPOINT_SUFFIX, PropertyValue::getValue, ADLSCredentialsDetails.Builder::setEndpointSuffix, attributes); | ||||||
| setValue(credentialsBuilder, USE_MANAGED_IDENTITY, PropertyValue::asBoolean, ADLSCredentialsDetails.Builder::setUseManagedIdentity, attributes); | ||||||
| setValue(credentialsBuilder, SERVICE_PRINCIPAL_TENANT_ID, PropertyValue::getValue, ADLSCredentialsDetails.Builder::setServicePrincipalTenantId, attributes); | ||||||
| setValue(credentialsBuilder, SERVICE_PRINCIPAL_CLIENT_ID, PropertyValue::getValue, ADLSCredentialsDetails.Builder::setServicePrincipalClientId, attributes); | ||||||
| setValue(credentialsBuilder, SERVICE_PRINCIPAL_CLIENT_SECRET, PropertyValue::getValue, ADLSCredentialsDetails.Builder::setServicePrincipalClientSecret, attributes); | ||||||
|
|
||||||
| return credentialsBuilder.build(); | ||||||
| } | ||||||
|
|
||||||
| private <T> void setValue( | ||||||
| ADLSCredentialsDetails.Builder credentialsBuilder, | ||||||
| PropertyDescriptor propertyDescriptor, Function<PropertyValue, T> getPropertyValue, | ||||||
| BiConsumer<ADLSCredentialsDetails.Builder, T> setBuilderValue | ||||||
| ADLSCredentialsDetails.Builder credentialsBuilder, | ||||||
| PropertyDescriptor propertyDescriptor, Function<PropertyValue, T> getPropertyValue, | ||||||
| BiConsumer<ADLSCredentialsDetails.Builder, T> setBuilderValue, Map<String, String> attributes | ||||||
| ) { | ||||||
| PropertyValue property = context.getProperty(propertyDescriptor); | ||||||
|
|
||||||
| if (property.isSet()) { | ||||||
| if (propertyDescriptor.isExpressionLanguageSupported()) { | ||||||
| property = property.evaluateAttributeExpressions(attributes); | ||||||
| } | ||||||
| T value = getPropertyValue.apply(property); | ||||||
| setBuilderValue.accept(credentialsBuilder, value); | ||||||
| } | ||||||
|
|
||||||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to use
displayName()here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Account KeyandSAS Tokenwould be fine but I would simply useManaged Identity(without the "Use" prefix from the displayname).I think it is something we should rather fix when the new
Authentication Typeproperty is added (which will have the same AllowableValue-s that would be needed here too).