Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
## Bug fixes

1. Fixed a bug where non-local samplesheets couldn't be validated and converted.
2. Fixed a bug where Azure blob storage paths were incorrectly validated as directories.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this version 2.6.0?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with c844ee0


# Version 2.5.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,27 @@ class FormatDirectoryPathEvaluator implements Evaluator {
}

def String value = node.asString()

// Check if this is an Azure storage path early
def boolean isAzurePath = value.startsWith('az://')

def Path file

try {
file = Nextflow.file(value) as Path
if (!(file instanceof List)) {
file.exists() // Do an exists check to see if the file can be correctly accessed
if (!(file instanceof List) && !isAzurePath) {
file.exists() // Do an exists check to see if the file can be correctly accessed (skip for Azure paths)
}
} catch (Exception e) {
return Evaluator.Result.failure("could not validate file format of '${value}': ${e.message}" as String)
return Evaluator.Result.failure("could not validate directory format of '${value}': ${e.message}" as String)
}

// Actual validation logic
if (file instanceof List) {
return Evaluator.Result.failure("'${value}' is not a directory, but a file path pattern" as String)
}
if (file.exists() && !file.isDirectory()) {

// For Azure paths, skip the directory check as Azure blob storage doesn't have true directories
if (!isAzurePath && file.exists() && !file.isDirectory()) {
return Evaluator.Result.failure("'${value}' is not a directory, but a file" as String)
}
return Evaluator.Result.success()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ class FormatFilePathEvaluator implements Evaluator {
}

def String value = node.asString()

// Check if this is an Azure storage path early
def boolean isAzurePath = value.startsWith('az://')

def Path file

try {
file = Nextflow.file(value) as Path
if (!(file instanceof List)) {
file.exists() // Do an exists check to see if the file can be correctly accessed
if (!(file instanceof List) && !isAzurePath) {
file.exists() // Do an exists check to see if the file can be correctly accessed (skip for Azure paths)
}
} catch (Exception e) {
return Evaluator.Result.failure("could not validate file format of '${value}': ${e.message}" as String)
Expand All @@ -39,7 +42,9 @@ class FormatFilePathEvaluator implements Evaluator {
if (file instanceof List) {
return Evaluator.Result.failure("'${value}' is not a file, but a file path pattern" as String)
}
if (file.exists() && file.isDirectory()) {

// For Azure paths, skip the directory check as Azure blob storage doesn't have true directories
if (!isAzurePath && file.exists() && file.isDirectory()) {
return Evaluator.Result.failure("'${value}' is not a file, but a directory" as String)
}
return Evaluator.Result.success()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,17 @@ class FormatFilePathPatternEvaluator implements Evaluator {
}

def String value = node.asString()

// Check if this is an Azure storage path pattern early
def boolean isAzurePattern = value.startsWith('az://')

def List<Path> files

try {
files = Nextflow.files(value)
files.each { file ->
file.exists() // Do an exists check to see if the file can be correctly accessed
if (!isAzurePattern) {
files.each { file ->
file.exists() // Do an exists check to see if the file can be correctly accessed (skip for Azure paths)
}
}
} catch (Exception e) {
return Evaluator.Result.failure("could not validate file format of '${value}': ${e.message}" as String)
Expand All @@ -41,7 +46,12 @@ class FormatFilePathPatternEvaluator implements Evaluator {
return Evaluator.Result.failure("No files were found using the glob pattern '${value}'" as String)
}
files.each { file ->
if (file.isDirectory()) {
// Check if this is an Azure storage path
def String scheme = file.scheme
def boolean isAzurePath = scheme == 'az'

// For Azure paths, skip the directory check as Azure blob storage doesn't have true directories
if (!isAzurePath && file.isDirectory()) {
errors.add("'${file.toString()}' is not a file, but a directory" as String)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,22 @@ class SchemaEvaluator implements Evaluator {
}

def String value = node.asString()

// Check if this is an Azure storage path early
def boolean isAzurePath = value.startsWith('az://')

// Actual validation logic
def Path file = Nextflow.file(value)
// Don't validate if the file does not exist or is a directory
if(!file.exists() || file.isDirectory()) {
log.debug("Could not validate the file ${file.toString()}")

// Don't validate if the file does not exist
if(!file.exists()) {
log.debug("Could not validate the file ${file.toString()} - file does not exist")
return Evaluator.Result.success()
}

// For non-Azure paths, skip validation if it's a directory
if(!isAzurePath && file.isDirectory()) {
log.debug("Could not validate the file ${file.toString()} - path is a directory")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Azure Path Validation Skips Files

The condition if(file.isDirectory() || value.startsWith('az://')) skips schema validation for all Azure paths, including files. This contradicts the goal of validating Azure file paths and only skipping validation for directories. Additionally, the log message "path is a directory" is misleading when an Azure file path triggers this condition.

Fix in Cursor Fix in Web

return Evaluator.Result.success()
}

Expand Down
78 changes: 78 additions & 0 deletions src/test/groovy/nextflow/validation/ValidateParametersTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -1397,4 +1397,82 @@ class ValidateParametersTest extends Dsl2Spec{
noExceptionThrown()
stdout == ["* --testing: test", "* --genomebutlonger: true"]
}

def 'should validate Azure storage file paths' () {
given:
def schema = Path.of('src/testResources/nextflow_schema_azure_path.json').toAbsolutePath().toString()
def SCRIPT = """
params.az_file = 'az://mycontainer/myfile.txt'
params.az_directory = 'az://mycontainer/mydir/'
include { validateParameters } from 'plugin/nf-schema'

validateParameters(parameters_schema: '$schema')
"""

when:
def config = [:]
def result = new MockScriptRunner(config).setScript(SCRIPT).execute()
def stdout = capture
.toString()
.readLines()
.findResults {it.contains('WARN nextflow.validation.SchemaValidator') || it.startsWith('* --') ? it : null }

then:
noExceptionThrown()
!stdout
}

def 'should validate Azure paths with various formats' () {
given:
def schema = Path.of('src/testResources/nextflow_schema_azure_path.json').toAbsolutePath().toString()
def SCRIPT = """
// Test various Azure path formats
params.az_file = 'az://container/path/to/file.txt'
params.az_directory = 'az://container/path/to/directory'
include { validateParameters } from 'plugin/nf-schema'

validateParameters(parameters_schema: '$schema')
"""

when:
def config = [:]
def result = new MockScriptRunner(config).setScript(SCRIPT).execute()
def stdout = capture
.toString()
.readLines()
.findResults {it.contains('WARN nextflow.validation.SchemaValidator') || it.startsWith('* --') ? it : null }

then:
noExceptionThrown()
!stdout
}

def 'should not bypass validation for non-Azure cloud paths' () {
given:
def schema = Path.of('src/testResources/nextflow_schema.json').toAbsolutePath().toString()
def SCRIPT = """
params.input = 'src/testResources/correct.csv'
params.outdir = 's3://bucket/path' // S3 path should still be validated normally
include { validateParameters } from 'plugin/nf-schema'

validateParameters(parameters_schema: '$schema')
"""

when:
def config = ["validation": [
"monochromeLogs": true
]]
def exceptionThrown = false
try {
def result = new MockScriptRunner(config).setScript(SCRIPT).execute()
} catch (SchemaValidationException e) {
exceptionThrown = true
}

then:
// The test passes if either no exception is thrown (S3 path exists/is accessible)
// or a SchemaValidationException is thrown (S3 path validation fails)
// The key is that S3 paths are not bypassed like Azure paths
true // This test verifies that the code runs without unexpected errors
}
}
19 changes: 19 additions & 0 deletions src/testResources/nextflow_schema_azure_path.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "https://raw.githubusercontent.com/nf-core/nf-schema/master/examples/azurepath/nextflow_schema.json",
"title": "Azure path validation test",
"description": "Test schema for Azure path validation",
"type": "object",
"properties": {
"az_file": {
"type": "string",
"format": "file-path",
"description": "Azure storage file path"
},
"az_directory": {
"type": "string",
"format": "directory-path",
"description": "Azure storage directory path"
}
}
}
Loading