hardened parsing of DatabaseEntities, validation moved to operation

create-data.cql

@@ -1,5 +1,5 @@
-CREATE KEYSPACE IF NOT EXISTS test1 WITH replication = {'class': 'NetworkTopologyStrategy', 'dc1': 3};
-CREATE KEYSPACE IF NOT EXISTS test2 WITH replication = {'class': 'NetworkTopologyStrategy', 'dc1': 3};
+CREATE KEYSPACE IF NOT EXISTS test1 WITH replication = {'class': 'NetworkTopologyStrategy', 'dc1': 2};
+CREATE KEYSPACE IF NOT EXISTS test2 WITH replication = {'class': 'NetworkTopologyStrategy', 'dc1': 2};
 
 CREATE TABLE IF NOT EXISTS test1.testtable1 ( id uuid primary key);
 CREATE TABLE IF NOT EXISTS test1.testtable2 ( id uuid primary key);

src/main/java/com/instaclustr/esop/impl/AbstractOperationRequest.java

@@ -1,6 +1,9 @@
 package com.instaclustr.esop.impl;
 
-import javax.validation.constraints.NotNull;
+import static java.lang.String.format;
+
+import java.util.Arrays;
+import java.util.Set;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonProperty;
@@ -9,7 +12,6 @@
 import com.instaclustr.esop.impl.StorageLocation.StorageLocationDeserializer;
 import com.instaclustr.esop.impl.StorageLocation.StorageLocationSerializer;
 import com.instaclustr.esop.impl.StorageLocation.StorageLocationTypeConverter;
-import com.instaclustr.esop.impl.StorageLocation.ValidStorageLocation;
 import com.instaclustr.esop.impl.retry.RetrySpec;
 import com.instaclustr.kubernetes.KubernetesSecretsReader;
 import com.instaclustr.operations.OperationRequest;
@@ -28,8 +30,6 @@ public abstract class AbstractOperationRequest extends OperationRequest {
             "cloudProvider://bucketName/clusterId/datacenterId/nodeId or file:///some/path/bucketName/clusterId/datacenterId/nodeId. " +
             "'cloudProvider' is one of 's3', 'oracle', 'azure' or 'gcp'.",
         required = true)
-    @NotNull
-    @ValidStorageLocation
     @JsonSerialize(using = StorageLocationSerializer.class)
     @JsonDeserialize(using = StorageLocationDeserializer.class)
     public StorageLocation storageLocation;
@@ -68,7 +68,7 @@ public AbstractOperationRequest() {
         // for picocli
     }
 
-    public AbstractOperationRequest(@NotNull final StorageLocation storageLocation,
+    public AbstractOperationRequest(final StorageLocation storageLocation,
                                     final String k8sNamespace,
                                     final String k8sSecretName,
                                     final boolean insecure,
@@ -113,4 +113,24 @@ public String resolveKubernetesNamespace() {
 
         return resolvedNamespace;
     }
+
+    public void validate(final Set<String> storageProviders) {
+        if (storageLocation == null) {
+            throw new IllegalStateException("storageLocation has to be specified!");
+        }
+
+        if (retry != null) {
+            retry.validate();
+        }
+
+        try {
+            storageLocation.validate();
+        } catch (Exception ex) {
+            throw new IllegalStateException(format("Invalid storage location: %s", ex.getLocalizedMessage()));
+        }
+
+        if (storageProviders != null && !storageProviders.contains(storageLocation.storageProvider)) {
+            throw new IllegalStateException(format("Available storage providers: %s", Arrays.toString(storageProviders.toArray())));
+        }
+    }
 }

src/main/java/com/instaclustr/esop/impl/DatabaseEntities.java

@@ -8,6 +8,8 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.core.JsonGenerator;
@@ -141,9 +143,8 @@ public static DatabaseEntities parse(final String entities) {
             return DatabaseEntities.empty();
         }
 
-        final String sanitizedEntities = entities.replaceAll("[ ]+", "");
-
-        final String[] keyspaceTablePairs = sanitizedEntities.split(",");
+        final String sanitized = entities.trim().replaceAll("[ ]+\\.", ".").replaceAll("\\.[ ]+", ".").replaceAll(" ", ",").replaceAll("[,]+", ",");
+        final String[] keyspaceTablePairs = Stream.of(sanitized.split(",")).filter(s -> !s.isEmpty()).toArray(String[]::new);
 
         final List<String> keyspaces = new ArrayList<>();
         final Multimap<String, String> keyspacesWithTables = HashMultimap.create();

src/main/java/com/instaclustr/esop/impl/RenamedEntities.java

@@ -38,7 +38,7 @@ public boolean areEmpty() {
         return this.renamed.isEmpty();
     }
 
-    public static void validate(final Map<String, String> rename) throws Exception {
+    public static void validate(final Map<String, String> rename) {
         if (rename == null) {
             return;
         }

src/main/java/com/instaclustr/esop/impl/StorageLocation.java

@@ -1,22 +1,10 @@
 package com.instaclustr.esop.impl;
 
 import static java.lang.String.format;
-import static java.lang.annotation.ElementType.FIELD;
-import static java.lang.annotation.ElementType.PARAMETER;
-import static java.lang.annotation.ElementType.TYPE;
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
-
-import javax.validation.Constraint;
-import javax.validation.ConstraintValidator;
-import javax.validation.ConstraintValidatorContext;
-import javax.validation.Payload;
+
 import java.io.IOException;
-import java.lang.annotation.Retention;
-import java.lang.annotation.Target;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.Arrays;
-import java.util.Set;
 import java.util.UUID;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -29,8 +17,6 @@
 import com.fasterxml.jackson.databind.exc.InvalidFormatException;
 import com.fasterxml.jackson.databind.ser.std.StdSerializer;
 import com.google.common.base.MoreObjects;
-import com.google.inject.Inject;
-import com.instaclustr.esop.guice.StorageProviders;
 import picocli.CommandLine;
 import picocli.CommandLine.ITypeConverter;
 
@@ -62,7 +48,7 @@ public StorageLocation(final String rawLocation) {
             initializeFileBackupLocation(this.rawLocation);
         } else {
             cloudLocation = true;
-            initializeCloudBackupLocation(this.rawLocation);
+            initializeCloudLocation(this.rawLocation);
         }
     }
 
@@ -86,7 +72,7 @@ private void initializeFileBackupLocation(final String backupLocation) {
         }
     }
 
-    private void initializeCloudBackupLocation(final String storageLocation) {
+    private void initializeCloudLocation(final String storageLocation) {
 
         final Matcher globalMatcher = globalPattern.matcher(storageLocation);
 
@@ -115,14 +101,14 @@ public void validate() throws IllegalStateException {
         if (cloudLocation) {
             if (!globalRequest) {
                 if (rawLocation == null || storageProvider == null || bucket == null || clusterId == null || datacenterId == null || nodeId == null) {
-                    throw new IllegalStateException(format("Backup location %s is not in form protocol://bucketName/clusterId/datacenterid/nodeId",
+                    throw new IllegalStateException(format("Storage location %s is not in form protocol://bucketName/clusterId/datacenterid/nodeId",
                                                            rawLocation));
                 }
             } else if (rawLocation == null || storageProvider == null || bucket == null) {
                 throw new IllegalStateException(format("Global storage location %s is not in form protocol://bucketName", rawLocation));
             }
         } else if (rawLocation == null || storageProvider == null || bucket == null || clusterId == null || datacenterId == null || nodeId == null || fileBackupDirectory == null) {
-            throw new IllegalStateException(format("Backup location %s is not in form file:///some/backup/path/clusterId/datacenterId/nodeId",
+            throw new IllegalStateException(format("Storage location %s is not in form file:///some/backup/path/clusterId/datacenterId/nodeId",
                                                    rawLocation));
         }
 
@@ -192,55 +178,6 @@ public String toString() {
             .toString();
     }
 
-    @Target({TYPE, PARAMETER, FIELD})
-    @Retention(RUNTIME)
-    @Constraint(validatedBy = ValidStorageLocation.StorageLocationValidator.class)
-    public @interface ValidStorageLocation {
-
-        String message() default "{com.instaclustr.esop.impl.StorageLocation.StorageLocationValidator.message}";
-
-        Class<?>[] groups() default {};
-
-        Class<? extends Payload>[] payload() default {};
-
-        class StorageLocationValidator implements ConstraintValidator<ValidStorageLocation, StorageLocation> {
-
-            private final Set<String> storageProviders;
-
-            @Inject
-            public StorageLocationValidator(final @StorageProviders Set<String> storageProviders) {
-                this.storageProviders = storageProviders;
-            }
-
-            @Override
-            public boolean isValid(final StorageLocation value, final ConstraintValidatorContext context) {
-
-                if (value == null) {
-                    return true;
-                }
-
-                context.disableDefaultConstraintViolation();
-
-                try {
-                    value.validate();
-                } catch (Exception ex) {
-                    context.buildConstraintViolationWithTemplate(format("Invalid backup location: %s",
-                                                                        ex.getLocalizedMessage())).addConstraintViolation();
-                    return false;
-                }
-
-                if (!storageProviders.contains(value.storageProvider)) {
-                    context.buildConstraintViolationWithTemplate(format("Available providers: %s",
-                                                                        Arrays.toString(storageProviders.toArray()))).addConstraintViolation();
-
-                    return false;
-                }
-
-                return true;
-            }
-        }
-    }
-
     public static class StorageLocationTypeConverter implements ITypeConverter<StorageLocation> {
 
         @Override

src/main/java/com/instaclustr/esop/impl/_import/ImportOperation.java

@@ -4,7 +4,6 @@
 import static java.lang.String.format;
 import static java.nio.file.Files.exists;
 
-import javax.validation.constraints.NotNull;
 import java.nio.file.Path;
 import java.time.Instant;
 import java.util.List;
@@ -64,7 +63,6 @@ private ImportOperation(@JsonProperty("type") final String type,
                             @JsonProperty("noInvalidateCaches") final boolean noInvalidateCaches,
                             @JsonProperty("quick") final boolean quick,
                             @JsonProperty("extendedVerify") final boolean extendedVerify,
-                            @NotNull
                             @JsonProperty("sourceDir")
                             @JsonDeserialize(using = NioPathDeserializer.class)
                             @JsonSerialize(using = NioPathSerializer.class) final Path sourceDir) {

src/main/java/com/instaclustr/esop/impl/_import/ImportOperationRequest.java

@@ -1,6 +1,5 @@
 package com.instaclustr.esop.impl._import;
 
-import javax.validation.constraints.NotNull;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 
@@ -134,7 +133,6 @@ public class ImportOperationRequest extends OperationRequest {
         description = "upon import, run an extended verify, verifying all values in the new sstables")
     public boolean extendedVerify = false;
 
-    @NotNull
     @JsonDeserialize(using = NioPathDeserializer.class)
     @JsonSerialize(using = NioPathSerializer.class)
     @Option(names = {"--import-source-dir"},
@@ -176,7 +174,6 @@ public ImportOperationRequest(@JsonProperty("type") final String type,
                                   @JsonProperty("noInvalidateCaches") final boolean noInvalidateCaches,
                                   @JsonProperty("quick") final boolean quick,
                                   @JsonProperty("extendedVerify") final boolean extendedVerify,
-                                  @NotNull
                                   @JsonProperty("sourceDir")
                                   @JsonDeserialize(using = NioPathDeserializer.class)
                                   @JsonSerialize(using = NioPathSerializer.class) final Path sourceDir) {

src/main/java/com/instaclustr/esop/impl/backup/BackupCommitLogsOperationRequest.java

@@ -1,9 +1,13 @@
 package com.instaclustr.esop.impl.backup;
 
+import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Set;
 
 import com.amazonaws.services.s3.model.MetadataDirective;
 import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
@@ -13,12 +17,12 @@
 import com.instaclustr.esop.impl.retry.RetrySpec;
 import com.instaclustr.jackson.PathDeserializer;
 import com.instaclustr.jackson.PathSerializer;
+import com.instaclustr.kubernetes.KubernetesHelper;
 import com.instaclustr.measure.DataRate;
 import com.instaclustr.measure.Time;
 import com.instaclustr.picocli.typeconverter.PathTypeConverter;
 import picocli.CommandLine.Option;
 
-@ValidBackupCommitLogsOperationRequest
 public class BackupCommitLogsOperationRequest extends BaseBackupOperationRequest {
 
     @Option(names = {"--cl-archive"},
@@ -103,4 +107,20 @@ public String toString() {
             .add("retry", retry)
             .toString();
     }
+
+    @JsonIgnore
+    public void validate(final Set<String> storageProviders) {
+        super.validate(storageProviders);
+        if (this.cassandraDirectory == null || this.cassandraDirectory.toFile().getAbsolutePath().equals("/")) {
+            this.cassandraDirectory = Paths.get("/var/lib/cassandra");
+        }
+
+        if (!Files.exists(this.cassandraDirectory)) {
+            throw new IllegalStateException(String.format("cassandraDirectory %s does not exist", cassandraDirectory));
+        }
+
+        if (KubernetesHelper.isRunningInKubernetes() && this.resolveKubernetesSecretName() == null) {
+            throw new IllegalStateException("This code is running in Kubernetes but there is not 'k8sSecretName' field set on backup request!");
+        }
+    }
 }

src/main/java/com/instaclustr/esop/impl/backup/BackupOperation.java

@@ -1,10 +1,10 @@
 package com.instaclustr.esop.impl.backup;
 
-import javax.validation.constraints.Min;
 import java.nio.file.Path;
 import java.time.Instant;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -15,6 +15,7 @@
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 import com.google.inject.assistedinject.Assisted;
 import com.google.inject.assistedinject.AssistedInject;
+import com.instaclustr.esop.guice.StorageProviders;
 import com.instaclustr.esop.impl.DatabaseEntities;
 import com.instaclustr.esop.impl.DatabaseEntities.DatabaseEntitiesDeserializer;
 import com.instaclustr.esop.impl.DatabaseEntities.DatabaseEntitiesSerializer;
@@ -33,10 +34,12 @@ public class BackupOperation extends Operation<BackupOperationRequest> implement
 
     private static final Logger logger = LoggerFactory.getLogger(BackupOperation.class);
 
+    private final Set<String> storageProviders;
     private final OperationCoordinator<BackupOperationRequest> coordinator;
 
     @AssistedInject
     public BackupOperation(Optional<OperationCoordinator<BackupOperationRequest>> coordinator,
+                           @StorageProviders Set<String> storageProviders,
                            @Assisted final BackupOperationRequest request) {
         super(request);
 
@@ -45,11 +48,13 @@ public BackupOperation(Optional<OperationCoordinator<BackupOperationRequest>> co
         }
 
         this.coordinator = coordinator.get();
+        this.storageProviders = storageProviders;
     }
 
     public BackupOperation(final BackupOperationRequest request) {
         super(request);
         this.coordinator = null;
+        this.storageProviders = null;
         this.type = "backup";
     }
 
@@ -79,7 +84,7 @@ private BackupOperation(@JsonProperty("type") final String type,
                             @JsonProperty("k8sSecretName") final String k8sBackupSecretName,
                             @JsonProperty("globalRequest") final boolean globalRequest,
                             @JsonProperty("dc") final String dc,
-                            @JsonProperty("timeout") @Min(1) final Integer timeout,
+                            @JsonProperty("timeout") final Integer timeout,
                             @JsonProperty("insecure") final boolean insecure,
                             @JsonProperty("createMissingBucket") final boolean createMissingBucket,
                             @JsonProperty("skipBucketVerification") final boolean skipBucketVerification,
@@ -109,6 +114,7 @@ private BackupOperation(@JsonProperty("type") final String type,
                                                                                                      proxySettings,
                                                                                                      retry));
         coordinator = null;
+        storageProviders = null;
     }
 
     @Override
@@ -119,6 +125,8 @@ protected Object clone() throws CloneNotSupportedException {
     @Override
     protected void run0() throws Exception {
         assert coordinator != null;
+        assert storageProviders != null;
+        request.validate(storageProviders);
         coordinator.coordinate(this);
     }
 }