You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by st...@apache.org on 2021/06/08 21:11:49 UTC
[hadoop] branch branch-3.3 updated (0224917 -> 8f0ba9e)
This is an automated email from the ASF dual-hosted git repository.
stevel pushed a change to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/hadoop.git.
from 0224917 HDFS-16048. RBF: Print network topology on the router web (#3062)
new 4ac9123 HADOOP-17631. Configuration ${env.VAR:-FALLBACK} to eval FALLBACK when restrictSystemProps=true (#2977)
new 8f0ba9e HADOOP-17725. Improve error message for token providers in ABFS (#3041)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../java/org/apache/hadoop/conf/Configuration.java | 79 ++++++++++++++--------
.../org/apache/hadoop/conf/TestConfiguration.java | 56 +++++++++++++++
.../hadoop/fs/azurebfs/AbfsConfiguration.java | 50 +++++++++++---
.../fs/azurebfs/TestAccountConfiguration.java | 49 ++++++++++++--
4 files changed, 189 insertions(+), 45 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org
[hadoop] 01/02: HADOOP-17631. Configuration ${env.VAR:-FALLBACK} to
eval FALLBACK when restrictSystemProps=true (#2977)
Posted by st...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
stevel pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/hadoop.git
commit 4ac9123619e043414c7f2a31f5919bd616fe4209
Author: Steve Loughran <st...@cloudera.com>
AuthorDate: Tue Jun 8 21:56:40 2021 +0100
HADOOP-17631. Configuration ${env.VAR:-FALLBACK} to eval FALLBACK when restrictSystemProps=true (#2977)
Contributed by Steve Loughran.
Change-Id: I9b82109eddeb659c01896152cf603d458e2a04cd
---
.../java/org/apache/hadoop/conf/Configuration.java | 79 ++++++++++++++--------
.../org/apache/hadoop/conf/TestConfiguration.java | 56 +++++++++++++++
2 files changed, 106 insertions(+), 29 deletions(-)
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
index aedde6b..364521c 100755
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
@@ -1139,36 +1139,37 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
final String var = eval.substring(varBounds[SUB_START_IDX],
varBounds[SUB_END_IDX]);
String val = null;
- if (!restrictSystemProps) {
- try {
- if (var.startsWith("env.") && 4 < var.length()) {
- String v = var.substring(4);
- int i = 0;
- for (; i < v.length(); i++) {
- char c = v.charAt(i);
- if (c == ':' && i < v.length() - 1 && v.charAt(i + 1) == '-') {
- val = getenv(v.substring(0, i));
- if (val == null || val.length() == 0) {
- val = v.substring(i + 2);
- }
- break;
- } else if (c == '-') {
- val = getenv(v.substring(0, i));
- if (val == null) {
- val = v.substring(i + 1);
- }
- break;
+ try {
+ // evaluate system properties or environment variables even when
+ // the configuration is restricted -the restrictions are enforced
+ // in the getenv/getProperty calls
+ if (var.startsWith("env.") && 4 < var.length()) {
+ String v = var.substring(4);
+ int i = 0;
+ for (; i < v.length(); i++) {
+ char c = v.charAt(i);
+ if (c == ':' && i < v.length() - 1 && v.charAt(i + 1) == '-') {
+ val = getenv(v.substring(0, i));
+ if (val == null || val.length() == 0) {
+ val = v.substring(i + 2);
}
+ break;
+ } else if (c == '-') {
+ val = getenv(v.substring(0, i));
+ if (val == null) {
+ val = v.substring(i + 1);
+ }
+ break;
}
- if (i == v.length()) {
- val = getenv(v);
- }
- } else {
- val = getProperty(var);
}
- } catch (SecurityException se) {
- LOG.warn("Unexpected SecurityException in Configuration", se);
+ if (i == v.length()) {
+ val = getenv(v);
+ }
+ } else {
+ val = getProperty(var);
}
+ } catch (SecurityException se) {
+ LOG.warn("Unexpected SecurityException in Configuration", se);
}
if (val == null) {
val = getRaw(var);
@@ -1194,13 +1195,33 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
throw new IllegalStateException("Variable substitution depth too large: "
+ MAX_SUBST + " " + expr);
}
-
+
+ /**
+ * Get the environment variable value if
+ * {@link #restrictSystemProps} does not block this.
+ * @param name environment variable name.
+ * @return the value or null if either it is unset or access forbidden.
+ */
String getenv(String name) {
- return System.getenv(name);
+ if (!restrictSystemProps) {
+ return System.getenv(name);
+ } else {
+ return null;
+ }
}
+ /**
+ * Get a system property value if
+ * {@link #restrictSystemProps} does not block this.
+ * @param key property key
+ * @return the value or null if either it is unset or access forbidden.
+ */
String getProperty(String key) {
- return System.getProperty(key);
+ if (!restrictSystemProps) {
+ return System.getProperty(key);
+ } else {
+ return null;
+ }
}
/**
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
index a2273ef..263e40c 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
@@ -490,6 +490,62 @@ public class TestConfiguration {
}
}
+ /**
+ * Verify that when a configuration is restricted, environment
+ * variables and system properties will be unresolved.
+ * The fallback patterns for the variables are still parsed.
+ */
+ @Test
+ public void testRestrictedEnv() throws IOException {
+ // this test relies on env.PATH being set on all platforms a
+ // test run will take place on, and the java.version sysprop
+ // set in all JVMs.
+ // Restricted configurations will not get access to these values, so
+ // will either be unresolved or, for env vars with fallbacks: the fallback
+ // values.
+
+ conf.setRestrictSystemProperties(true);
+
+ out = new BufferedWriter(new FileWriter(CONFIG));
+ startConfig();
+ // a simple property to reference
+ declareProperty("d", "D", "D");
+
+ // system property evaluation stops working completely
+ declareProperty("system1", "${java.version}", "${java.version}");
+
+ // the env variable does not resolve
+ declareProperty("secret1", "${env.PATH}", "${env.PATH}");
+
+ // but all the fallback options do work
+ declareProperty("secret2", "${env.PATH-a}", "a");
+ declareProperty("secret3", "${env.PATH:-b}", "b");
+ declareProperty("secret4", "${env.PATH:-}", "");
+ declareProperty("secret5", "${env.PATH-}", "");
+ // special case
+ declareProperty("secret6", "${env.PATH:}", "${env.PATH:}");
+ // safety check
+ declareProperty("secret7", "${env.PATH:--}", "-");
+
+ // recursive eval of the fallback
+ declareProperty("secret8", "${env.PATH:-${d}}", "D");
+
+ // if the fallback doesn't resolve, the result is the whole variable raw.
+ declareProperty("secret9", "${env.PATH:-$d}}", "${env.PATH:-$d}}");
+
+ endConfig();
+ Path fileResource = new Path(CONFIG);
+ conf.addResource(fileResource);
+
+ for (Prop p : props) {
+ System.out.println("p=" + p.name);
+ String gotVal = conf.get(p.name);
+ String gotRawVal = conf.getRaw(p.name);
+ assertEq(p.val, gotRawVal);
+ assertEq(p.expectEval, gotVal);
+ }
+ }
+
@Test
public void testFinalParam() throws IOException {
out=new BufferedWriter(new FileWriter(CONFIG));
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org
[hadoop] 02/02: HADOOP-17725. Improve error message for token
providers in ABFS (#3041)
Posted by st...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
stevel pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/hadoop.git
commit 8f0ba9ee1b89901ba8961df3576a2b44f0e856cf
Author: Viraj Jasani <vj...@apache.org>
AuthorDate: Wed Jun 9 02:33:03 2021 +0530
HADOOP-17725. Improve error message for token providers in ABFS (#3041)
Contributed by Viraj Jasani.
---
.../hadoop/fs/azurebfs/AbfsConfiguration.java | 50 +++++++++++++++++-----
.../fs/azurebfs/TestAccountConfiguration.java | 49 ++++++++++++++++++---
2 files changed, 83 insertions(+), 16 deletions(-)
diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java
index 1c4a09b..82a116b 100644
--- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java
+++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java
@@ -403,6 +403,24 @@ public class AbfsConfiguration{
}
/**
+ * Returns a value for the key if the value exists and is not null.
+ * Otherwise, throws {@link ConfigurationPropertyNotFoundException} with
+ * key name.
+ *
+ * @param key Account-agnostic configuration key
+ * @return value if exists
+ * @throws IOException if error in fetching password or
+ * ConfigurationPropertyNotFoundException for missing key
+ */
+ private String getMandatoryPasswordString(String key) throws IOException {
+ String value = getPasswordString(key);
+ if (value == null) {
+ throw new ConfigurationPropertyNotFoundException(key);
+ }
+ return value;
+ }
+
+ /**
* Returns account-specific token provider class if it exists, else checks if
* an account-agnostic setting is present for token provider class if AuthType
* matches with authType passed.
@@ -742,25 +760,33 @@ public class AbfsConfiguration{
FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME, null,
AccessTokenProvider.class);
- AccessTokenProvider tokenProvider = null;
+ AccessTokenProvider tokenProvider;
if (tokenProviderClass == ClientCredsTokenProvider.class) {
- String authEndpoint = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT);
- String clientId = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID);
- String clientSecret = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_SECRET);
+ String authEndpoint =
+ getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT);
+ String clientId =
+ getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID);
+ String clientSecret =
+ getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_SECRET);
tokenProvider = new ClientCredsTokenProvider(authEndpoint, clientId, clientSecret);
LOG.trace("ClientCredsTokenProvider initialized");
} else if (tokenProviderClass == UserPasswordTokenProvider.class) {
- String authEndpoint = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT);
- String username = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_USER_NAME);
- String password = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_USER_PASSWORD);
+ String authEndpoint =
+ getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT);
+ String username =
+ getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_USER_NAME);
+ String password =
+ getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_USER_PASSWORD);
tokenProvider = new UserPasswordTokenProvider(authEndpoint, username, password);
LOG.trace("UserPasswordTokenProvider initialized");
} else if (tokenProviderClass == MsiTokenProvider.class) {
String authEndpoint = getTrimmedPasswordString(
FS_AZURE_ACCOUNT_OAUTH_MSI_ENDPOINT,
AuthConfigurations.DEFAULT_FS_AZURE_ACCOUNT_OAUTH_MSI_ENDPOINT);
- String tenantGuid = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_MSI_TENANT);
- String clientId = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID);
+ String tenantGuid =
+ getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_MSI_TENANT);
+ String clientId =
+ getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID);
String authority = getTrimmedPasswordString(
FS_AZURE_ACCOUNT_OAUTH_MSI_AUTHORITY,
AuthConfigurations.DEFAULT_FS_AZURE_ACCOUNT_OAUTH_MSI_AUTHORITY);
@@ -772,8 +798,10 @@ public class AbfsConfiguration{
String authEndpoint = getTrimmedPasswordString(
FS_AZURE_ACCOUNT_OAUTH_REFRESH_TOKEN_ENDPOINT,
AuthConfigurations.DEFAULT_FS_AZURE_ACCOUNT_OAUTH_REFRESH_TOKEN_ENDPOINT);
- String refreshToken = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_REFRESH_TOKEN);
- String clientId = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID);
+ String refreshToken =
+ getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_REFRESH_TOKEN);
+ String clientId =
+ getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID);
tokenProvider = new RefreshTokenBasedTokenProvider(authEndpoint,
clientId, refreshToken);
LOG.trace("RefreshTokenBasedTokenProvider initialized");
diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAccountConfiguration.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAccountConfiguration.java
index 4cb0961..86bb2ad 100644
--- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAccountConfiguration.java
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAccountConfiguration.java
@@ -19,18 +19,22 @@
package org.apache.hadoop.fs.azurebfs;
import java.io.IOException;
-
-import org.assertj.core.api.Assertions;
-import org.junit.Test;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.azurebfs.contracts.exceptions.ConfigurationPropertyNotFoundException;
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidConfigurationValueException;
+import org.apache.hadoop.fs.azurebfs.contracts.exceptions.TokenAccessProviderException;
import org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider;
import org.apache.hadoop.fs.azurebfs.oauth2.CustomTokenProviderAdapter;
import org.apache.hadoop.fs.azurebfs.services.AuthType;
+import org.apache.hadoop.test.GenericTestUtils;
+import org.apache.hadoop.test.LambdaTestUtils;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT;
@@ -38,6 +42,8 @@ import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_SECRET;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_SAS_TOKEN_PROVIDER_TYPE;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
/**
* Tests correct precedence of various configurations that might be returned.
@@ -60,6 +66,12 @@ public class TestAccountConfiguration {
private static final String TEST_CLIENT_ID = "clientId";
private static final String TEST_CLIENT_SECRET = "clientSecret";
+ private static final List<String> CONFIG_KEYS =
+ Collections.unmodifiableList(Arrays.asList(
+ FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT,
+ FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID,
+ FS_AZURE_ACCOUNT_OAUTH_CLIENT_SECRET));
+
@Test
public void testStringPrecedence()
throws IllegalAccessException, IOException, InvalidConfigurationValueException {
@@ -361,6 +373,33 @@ public class TestAccountConfiguration {
testGlobalAndAccountOAuthPrecedence(abfsConf, null, AuthType.OAuth);
}
+ @Test
+ public void testConfigPropNotFound() throws Throwable {
+ final String accountName = "account";
+
+ final Configuration conf = new Configuration();
+ final AbfsConfiguration abfsConf = new AbfsConfiguration(conf, accountName);
+
+ for (String key : CONFIG_KEYS) {
+ setAuthConfig(abfsConf, true, AuthType.OAuth);
+ abfsConf.unset(key + "." + accountName);
+ testMissingConfigKey(abfsConf, key);
+ }
+
+ unsetAuthConfig(abfsConf, false);
+ unsetAuthConfig(abfsConf, true);
+ }
+
+ private static void testMissingConfigKey(final AbfsConfiguration abfsConf,
+ final String confKey) throws Throwable {
+ GenericTestUtils.assertExceptionContains("Configuration property "
+ + confKey + " not found.",
+ LambdaTestUtils.verifyCause(
+ ConfigurationPropertyNotFoundException.class,
+ LambdaTestUtils.intercept(TokenAccessProviderException.class,
+ () -> abfsConf.getTokenProvider().getClass().getTypeName())));
+ }
+
public void testGlobalAndAccountOAuthPrecedence(AbfsConfiguration abfsConf,
AuthType globalAuthType,
AuthType accountSpecificAuthType)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org