This is an automated email from the ASF dual-hosted git repository.
georgew5656 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 40ba429c5f4 More validation for Azure account config (#16561)
40ba429c5f4 is described below
commit 40ba429c5f490ac457fd050f5c1fa04ab7f264de
Author: Andreas Maechler <[email protected]>
AuthorDate: Fri Jun 7 14:24:15 2024 -0600
More validation for Azure account config (#16561)
* Mark `account` as NotNull
* Remove account test
Handled by annotation now
* Cleanup account config
* Mark container as not-null.
---
.../druid/storage/azure/AzureAccountConfig.java | 155 ++++++++-------------
.../storage/azure/AzureDataSegmentConfig.java | 5 +-
.../storage/azure/AzureAccountConfigTest.java | 85 +++++++----
.../storage/azure/AzureClientFactoryTest.java | 10 +-
.../storage/azure/AzureStorageDruidModuleTest.java | 21 ---
5 files changed, 119 insertions(+), 157 deletions(-)
diff --git
a/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureAccountConfig.java
b/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureAccountConfig.java
index 99984bfe43e..938db5393eb 100644
---
a/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureAccountConfig.java
+++
b/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureAccountConfig.java
@@ -23,56 +23,50 @@ import com.fasterxml.jackson.annotation.JsonProperty;
import javax.annotation.Nullable;
import javax.validation.constraints.Min;
-import java.util.Objects;
+import javax.validation.constraints.NotNull;
/**
* Stores the configuration for an Azure account.
*/
public class AzureAccountConfig
{
- static final String DEFAULT_PROTOCOL = "https";
- static final int DEFAULT_MAX_TRIES = 3;
-
- @JsonProperty
- private String protocol = DEFAULT_PROTOCOL;
-
@JsonProperty
- @Min(1)
- private int maxTries = DEFAULT_MAX_TRIES;
+ @NotNull
+ private String account;
+ /**
+ * @deprecated Use {@link #storageAccountEndpointSuffix} instead.
+ */
+ @Deprecated
+ @Nullable
@JsonProperty
- private String account;
+ private String endpointSuffix = null;
@JsonProperty
private String key;
@JsonProperty
- private String sharedAccessStorageToken;
+ private String managedIdentityClientId;
@JsonProperty
- private String managedIdentityClientId;
+ @Min(1)
+ private int maxTries = 3;
@JsonProperty
- private Boolean useAzureCredentialsChain = Boolean.FALSE;
+ private String protocol = "https";
- @Deprecated
- @Nullable
@JsonProperty
- private String endpointSuffix = null;
+ private String sharedAccessStorageToken;
@JsonProperty
private String storageAccountEndpointSuffix =
AzureUtils.AZURE_STORAGE_HOST_ADDRESS;
- @SuppressWarnings("unused") // Used by Jackson deserialization?
- public void setProtocol(String protocol)
- {
- this.protocol = protocol;
- }
+ @JsonProperty
+ private boolean useAzureCredentialsChain = false;
- @SuppressWarnings("unused") // Used by Jackson deserialization?
- public void setMaxTries(int maxTries)
+ public String getAccount()
{
- this.maxTries = maxTries;
+ return account;
}
public void setAccount(String account)
@@ -80,137 +74,98 @@ public class AzureAccountConfig
this.account = account;
}
- @SuppressWarnings("unused") // Used by Jackson deserialization?
- public void setKey(String key)
+ @Nullable
+ @Deprecated
+ public String getEndpointSuffix()
{
- this.key = key;
+ return endpointSuffix;
}
- @SuppressWarnings("unused") // Used by Jackson deserialization?
public void setEndpointSuffix(String endpointSuffix)
{
this.endpointSuffix = endpointSuffix;
}
- @SuppressWarnings("unused") // Used by Jackson deserialization?
- public void setStorageAccountEndpointSuffix(String
storageAccountEndpointSuffix)
+ public String getKey()
{
- this.storageAccountEndpointSuffix = storageAccountEndpointSuffix;
+ return key;
}
- public String getProtocol()
+ public void setKey(String key)
{
- return protocol;
+ this.key = key;
}
- public int getMaxTries()
+ public String getManagedIdentityClientId()
{
- return maxTries;
+ return managedIdentityClientId;
}
- public String getAccount()
+ public void setManagedIdentityClientId(String managedIdentityClientId)
{
- return account;
+ this.managedIdentityClientId = managedIdentityClientId;
}
- public String getKey()
+ public int getMaxTries()
{
- return key;
+ return maxTries;
}
- public String getSharedAccessStorageToken()
+ public void setMaxTries(int maxTries)
{
- return sharedAccessStorageToken;
+ this.maxTries = maxTries;
}
- public Boolean getUseAzureCredentialsChain()
+ public String getProtocol()
{
- return useAzureCredentialsChain;
+ return protocol;
}
- public String getManagedIdentityClientId()
+ public void setProtocol(String protocol)
{
- return managedIdentityClientId;
+ this.protocol = protocol;
}
+ public String getSharedAccessStorageToken()
+ {
+ return sharedAccessStorageToken;
+ }
- @SuppressWarnings("unused") // Used by Jackson deserialization?
public void setSharedAccessStorageToken(String sharedAccessStorageToken)
{
this.sharedAccessStorageToken = sharedAccessStorageToken;
}
- @SuppressWarnings("unused") // Used by Jackson deserialization?
- public void setManagedIdentityClientId(String managedIdentityClientId)
+ public String getStorageAccountEndpointSuffix()
{
- this.managedIdentityClientId = managedIdentityClientId;
+ return storageAccountEndpointSuffix;
}
- public void setUseAzureCredentialsChain(Boolean useAzureCredentialsChain)
+ public void setStorageAccountEndpointSuffix(String
storageAccountEndpointSuffix)
{
- this.useAzureCredentialsChain = useAzureCredentialsChain;
+ this.storageAccountEndpointSuffix = storageAccountEndpointSuffix;
}
- @Nullable
- @Deprecated
- public String getEndpointSuffix()
+ public Boolean getUseAzureCredentialsChain()
{
- return endpointSuffix;
+ return useAzureCredentialsChain;
}
- public String getStorageAccountEndpointSuffix()
+ public void setUseAzureCredentialsChain(Boolean useAzureCredentialsChain)
{
- return storageAccountEndpointSuffix;
+ this.useAzureCredentialsChain = useAzureCredentialsChain;
}
+ /**
+ * Helper to support legacy runtime property. Replace with {@link
#getStorageAccountEndpointSuffix()} when
+ * deprecated endpointSuffix has been removed.
+ */
public String getBlobStorageEndpoint()
{
- // this is here to support the legacy runtime property.
if (endpointSuffix != null) {
return AzureUtils.BLOB + "." + endpointSuffix;
}
+
return storageAccountEndpointSuffix;
}
- @Override
- public boolean equals(Object o)
- {
- if (this == o) {
- return true;
- }
- if (o == null || getClass() != o.getClass()) {
- return false;
- }
- AzureAccountConfig that = (AzureAccountConfig) o;
- return Objects.equals(protocol, that.protocol)
- && Objects.equals(maxTries, that.maxTries)
- && Objects.equals(account, that.account)
- && Objects.equals(key, that.key)
- && Objects.equals(sharedAccessStorageToken,
that.sharedAccessStorageToken)
- && Objects.equals(managedIdentityClientId,
that.managedIdentityClientId)
- && Objects.equals(useAzureCredentialsChain,
that.useAzureCredentialsChain)
- && Objects.equals(endpointSuffix, that.endpointSuffix)
- && Objects.equals(storageAccountEndpointSuffix,
that.storageAccountEndpointSuffix);
- }
-
- @Override
- public int hashCode()
- {
- return Objects.hash(protocol, maxTries, account, key,
sharedAccessStorageToken, managedIdentityClientId, useAzureCredentialsChain,
endpointSuffix, storageAccountEndpointSuffix);
- }
-
- @Override
- public String toString()
- {
- return "AzureAccountConfig{" +
- "protocol=" + protocol +
- ", maxTries=" + maxTries +
- ", account=" + account +
- ", key=" + key +
- ", sharedAccessStorageToken=" + sharedAccessStorageToken +
- ", managedIdentityClientId=" + managedIdentityClientId +
- ", useAzureCredentialsChain=" + useAzureCredentialsChain +
- ", endpointSuffix=" + endpointSuffix +
- ", storageAccountEndpointSuffix=" + storageAccountEndpointSuffix +
- '}';
- }
}
diff --git
a/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentConfig.java
b/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentConfig.java
index 1272e24dd41..010b7691c98 100644
---
a/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentConfig.java
+++
b/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentConfig.java
@@ -21,12 +21,15 @@ package org.apache.druid.storage.azure;
import com.fasterxml.jackson.annotation.JsonProperty;
+import javax.validation.constraints.NotNull;
+
/**
- * Stores the configuration for segments written to Azure deep storage
+ * Stores the configuration for segments written to Azure deep storage.
*/
public class AzureDataSegmentConfig
{
@JsonProperty
+ @NotNull
private String container;
@JsonProperty
diff --git
a/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureAccountConfigTest.java
b/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureAccountConfigTest.java
index 27328e2a97b..077a39c3e1a 100644
---
a/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureAccountConfigTest.java
+++
b/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureAccountConfigTest.java
@@ -34,10 +34,8 @@ public class AzureAccountConfigTest
public void
test_getBlobStorageEndpoint_endpointSuffixNullAndStorageAccountEndpointSuffixNull_expectedDefault()
throws JsonProcessingException
{
- AzureAccountConfig config = new AzureAccountConfig();
- AzureAccountConfig configSerde = MAPPER.readValue("{}",
AzureAccountConfig.class);
- assertEquals(configSerde, config);
- assertEquals(AzureUtils.AZURE_STORAGE_HOST_ADDRESS,
config.getBlobStorageEndpoint());
+ AzureAccountConfig emptyConfig = MAPPER.readValue("{}",
AzureAccountConfig.class);
+ assertEquals(AzureUtils.AZURE_STORAGE_HOST_ADDRESS,
emptyConfig.getBlobStorageEndpoint());
}
@Test
@@ -45,15 +43,21 @@ public class AzureAccountConfigTest
throws JsonProcessingException
{
String endpointSuffix = "core.usgovcloudapi.net";
+
AzureAccountConfig config = new AzureAccountConfig();
config.setEndpointSuffix(endpointSuffix);
- AzureAccountConfig configSerde = MAPPER.readValue(
- "{"
- + "\"endpointSuffix\": \"" + endpointSuffix + "\""
- + "}",
- AzureAccountConfig.class);
- assertEquals(configSerde, config);
- assertEquals(AzureUtils.BLOB + "." + endpointSuffix,
config.getBlobStorageEndpoint());
+
+ assertEquals(
+ MAPPER.writeValueAsString(config),
+ MAPPER.writeValueAsString(
+ MAPPER.readValue(
+ "{"
+ + "\"endpointSuffix\": \"" + endpointSuffix + "\""
+ + "}",
+ AzureAccountConfig.class
+ )
+ )
+ );
}
@Test
@@ -62,17 +66,24 @@ public class AzureAccountConfigTest
{
String endpointSuffix = "core.usgovcloudapi.net";
String storageAccountEndpointSuffix = "ABCD1234.blob.storage.azure.net";
+
AzureAccountConfig config = new AzureAccountConfig();
config.setEndpointSuffix(endpointSuffix);
config.setStorageAccountEndpointSuffix(storageAccountEndpointSuffix);
- AzureAccountConfig configSerde = MAPPER.readValue(
- "{"
- + "\"endpointSuffix\": \"" + endpointSuffix + "\","
- + " \"storageAccountEndpointSuffix\": \"" +
storageAccountEndpointSuffix + "\""
- + "}",
- AzureAccountConfig.class);
- assertEquals(configSerde, config);
+
assertEquals(AzureUtils.BLOB + "." + endpointSuffix,
config.getBlobStorageEndpoint());
+ assertEquals(
+ MAPPER.writeValueAsString(config),
+ MAPPER.writeValueAsString(
+ MAPPER.readValue(
+ "{"
+ + "\"endpointSuffix\": \"" + endpointSuffix + "\","
+ + " \"storageAccountEndpointSuffix\": \"" +
storageAccountEndpointSuffix + "\""
+ + "}",
+ AzureAccountConfig.class
+ )
+ )
+ );
}
@Test
@@ -80,15 +91,22 @@ public class AzureAccountConfigTest
throws JsonProcessingException
{
String storageAccountEndpointSuffix = "ABCD1234.blob.storage.azure.net";
+
AzureAccountConfig config = new AzureAccountConfig();
config.setStorageAccountEndpointSuffix(storageAccountEndpointSuffix);
- AzureAccountConfig configSerde = MAPPER.readValue(
- "{"
- + "\"storageAccountEndpointSuffix\": \"" +
storageAccountEndpointSuffix + "\""
- + "}",
- AzureAccountConfig.class);
- assertEquals(configSerde, config);
+
assertEquals(storageAccountEndpointSuffix,
config.getBlobStorageEndpoint());
+ assertEquals(
+ MAPPER.writeValueAsString(config),
+ MAPPER.writeValueAsString(
+ MAPPER.readValue(
+ "{"
+ + "\"storageAccountEndpointSuffix\": \"" +
storageAccountEndpointSuffix + "\""
+ + "}",
+ AzureAccountConfig.class
+ )
+ )
+ );
}
@Test
@@ -96,14 +114,21 @@ public class AzureAccountConfigTest
throws JsonProcessingException
{
String managedIdentityClientId = "blah";
+
AzureAccountConfig config = new AzureAccountConfig();
config.setManagedIdentityClientId("blah");
- AzureAccountConfig configSerde = MAPPER.readValue(
- "{"
- + "\"managedIdentityClientId\": \"" + managedIdentityClientId + "\""
- + "}",
- AzureAccountConfig.class);
- assertEquals(configSerde, config);
+
assertEquals("blah", config.getManagedIdentityClientId());
+ assertEquals(
+ MAPPER.writeValueAsString(config),
+ MAPPER.writeValueAsString(
+ MAPPER.readValue(
+ "{"
+ + "\"managedIdentityClientId\": \"" + managedIdentityClientId
+ "\""
+ + "}",
+ AzureAccountConfig.class
+ )
+ )
+ );
}
}
diff --git
a/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureClientFactoryTest.java
b/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureClientFactoryTest.java
index 5c4d1c433c8..a19eff8eafa 100644
---
a/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureClientFactoryTest.java
+++
b/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureClientFactoryTest.java
@@ -135,7 +135,7 @@ public class AzureClientFactoryTest
AzureAccountConfig config = new AzureAccountConfig();
config.setKey("key");
azureClientFactory = new AzureClientFactory(config);
- BlobServiceClient expectedBlobServiceClient =
azureClientFactory.getBlobServiceClient(AzureAccountConfig.DEFAULT_MAX_TRIES,
ACCOUNT);
+ BlobServiceClient expectedBlobServiceClient =
azureClientFactory.getBlobServiceClient(3, ACCOUNT);
BlobServiceClient blobServiceClient =
azureClientFactory.getBlobServiceClient(null, ACCOUNT);
assertEquals(expectedBlobServiceClient, blobServiceClient);
}
@@ -148,7 +148,7 @@ public class AzureClientFactoryTest
AzureAccountConfig config = new AzureAccountConfig();
config.setKey("key");
config.setEndpointSuffix(endpointSuffix);
- URL expectedAccountUrl = new URL(AzureAccountConfig.DEFAULT_PROTOCOL,
ACCOUNT + "." + AzureUtils.BLOB + "." + endpointSuffix, "");
+ URL expectedAccountUrl = new URL("https", ACCOUNT + "." + AzureUtils.BLOB
+ "." + endpointSuffix, "");
azureClientFactory = new AzureClientFactory(config);
BlobServiceClient blobServiceClient =
azureClientFactory.getBlobServiceClient(null, ACCOUNT);
assertEquals(expectedAccountUrl.toString(),
blobServiceClient.getAccountUrl());
@@ -164,7 +164,7 @@ public class AzureClientFactoryTest
config.setKey("key");
config.setEndpointSuffix(endpointSuffix);
config.setStorageAccountEndpointSuffix(storageAccountEndpointSuffix);
- URL expectedAccountUrl = new URL(AzureAccountConfig.DEFAULT_PROTOCOL,
ACCOUNT + "." + AzureUtils.BLOB + "." + endpointSuffix, "");
+ URL expectedAccountUrl = new URL("https", ACCOUNT + "." + AzureUtils.BLOB
+ "." + endpointSuffix, "");
azureClientFactory = new AzureClientFactory(config);
BlobServiceClient blobServiceClient =
azureClientFactory.getBlobServiceClient(null, ACCOUNT);
assertEquals(expectedAccountUrl.toString(),
blobServiceClient.getAccountUrl());
@@ -178,7 +178,7 @@ public class AzureClientFactoryTest
AzureAccountConfig config = new AzureAccountConfig();
config.setKey("key");
config.setStorageAccountEndpointSuffix(storageAccountEndpointSuffix);
- URL expectedAccountUrl = new URL(AzureAccountConfig.DEFAULT_PROTOCOL,
ACCOUNT + "." + storageAccountEndpointSuffix, "");
+ URL expectedAccountUrl = new URL("https", ACCOUNT + "." +
storageAccountEndpointSuffix, "");
azureClientFactory = new AzureClientFactory(config);
BlobServiceClient blobServiceClient =
azureClientFactory.getBlobServiceClient(null, ACCOUNT);
assertEquals(expectedAccountUrl.toString(),
blobServiceClient.getAccountUrl());
@@ -203,7 +203,7 @@ public class AzureClientFactoryTest
config.setStorageAccountEndpointSuffix(storageAccountEndpointSuffix);
final AzureClientFactory localAzureClientFactory = new
AzureClientFactory(config);
final URL expectedAccountUrl = new URL(
- AzureAccountConfig.DEFAULT_PROTOCOL,
+ "https",
ACCOUNT + "." + storageAccountEndpointSuffix,
""
);
diff --git
a/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureStorageDruidModuleTest.java
b/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureStorageDruidModuleTest.java
index ab1ab379a8a..22da901cf0b 100644
---
a/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureStorageDruidModuleTest.java
+++
b/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureStorageDruidModuleTest.java
@@ -256,27 +256,6 @@ public class AzureStorageDruidModuleTest extends
EasyMockSupport
);
}
- @Test
- public void testAccountUnset()
- {
- final Properties properties = initializePropertes();
- properties.remove("druid.azure.account");
-
- final ProvisionException exception = assertThrows(
- ProvisionException.class,
- () -> makeInjectorWithProperties(properties).getInstance(
- Key.get(new TypeLiteral<AzureClientFactory>()
- {
- })
- )
- );
-
- assertEquals(
- "Set 'account' to the storage account that needs to be configured in
the azure config. Please refer to azure documentation.",
- exception.getCause().getMessage()
- );
- }
-
@Test
public void testGetBlobStorageEndpointWithDefaultProperties()
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
