More validation for Azure account config (apache#16561)

* Mark `account` as NotNull * Remove account test Handled by annotation now * Cleanup account config * Mark container as not-null.
kfaraz · Jun 7, 2024 · 40ba429 · 40ba429
1 parent e6a82e8
commit 40ba429
Show file tree

Hide file tree

Showing 5 changed files with 119 additions and 157 deletions.
diff --git a/...ore/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureAccountConfig.java b/...ore/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureAccountConfig.java
@@ -23,194 +23,149 @@
 
 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)
   {
     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/...azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentConfig.java b/...azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentConfig.java
@@ -21,12 +21,15 @@
 
 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/...azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureAccountConfigTest.java b/...azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureAccountConfigTest.java
@@ -34,26 +34,30 @@ 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
   public void test_getBlobStorageEndpoint_endpointSuffixNotNullAndStorageAccountEndpointSuffixNull_expectEndpoint()
       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,48 +66,69 @@ public void test_getBlobStorageEndpoint_endpointSuffixNotNullAndStorageAccountEn
   {
     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
   public void test_getBlobStorageEndpoint_endpointSuffixNullAndStorageAccountEndpointSuffixNotNull_expectStorageAccountEndpoint()
       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
   public void test_getManagedIdentityClientId_withValueForManagedIdentityClientId_expectManagedIdentityClientId()
       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
+            )
+        )
+    );
   }
 }