opensource/galaxy
3
0
Fork 0
mirror of https://github.com/galaxyproject/galaxy.git synced 2026-09-24 16:30:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

Clarifies optional variable and secret handling

Browse Source 
Updates logic to require variables and secrets unless explicitly marked as optional, regardless of the presence of default values. Ensures validators run only when a value is supplied or a default is applied, and improves test coverage to reflect the new behavior. Prevents defaults from implicitly making fields optional, reducing ambiguity and enforcing clearer schema intent.
This commit is contained in:
davelopez
2026-02-11 10:04:10 +01:00
parent 9dc42fed9c
commit 5372c5433d
2 changed files with 125 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/galaxy/util/config_templates.py
+20 -9
View File
@@ -115,7 +115,6 @@ class TemplateSecret(StrictModel):
label: Optional[str] = None
help: Optional[MarkdownContent] = None
optional: Optional[bool] = None
default: Optional[str] = None # If set, secret is optional
class TemplateEnvironmentSecret(StrictModel):
@@ -179,7 +178,8 @@ def populate_default_variables(variables: Optional[List[TemplateVariable]], vari
if variables:
for variable in variables:
name = variable.name
if name not in variable_values and variable.default is not None:
# Apply defaults only for explicitly optional variables
if variable.optional and name not in variable_values and variable.default is not None:
variable_values[name] = variable.default
@@ -365,8 +365,8 @@ def validate_defines_all_required_secrets(instance: InstanceDefinition, template
secrets = instance.secrets
for template_secret in template.secrets or []:
name = template_secret.name
has_default = template_secret.default is not None
if name not in secrets and not has_default:
is_optional = bool(template_secret.optional)
if name not in secrets and not is_optional:
raise RequestParameterMissingException(f"Must define secret '{name}'")
@@ -374,8 +374,8 @@ def validate_defines_all_required_variables(instance: InstanceDefinition, templa
variables = instance.variables
for template_variable in template.variables or []:
name = template_variable.name
has_default = template_variable.default is not None
if name not in variables and not has_default:
is_optional = bool(template_variable.optional)
if name not in variables and not is_optional:
raise RequestParameterMissingException(f"Must define variable '{name}'")
@@ -391,7 +391,18 @@ def validate_specified_datatypes(instance: InstanceDefinition, template: Templat
def validate_specified_datatypes_variables(variables: Dict[str, Any], template: Template):
for template_variable in template.variables or []:
name = template_variable.name
variable_value = variables.get(name, template_variable.default)
# Only fall back to default for optional variables
if name in variables:
variable_value = variables[name]
elif template_variable.optional:
variable_value = template_variable.default
else:
variable_value = None
# Skip validation only if variable is optional, not provided, and has no default applied
if name not in variables and template_variable.optional and template_variable.default is None:
continue
template_type = template_variable.type
if template_type in ["string", "path_component"]:
if not isinstance(variable_value, str):
@@ -412,8 +423,8 @@ def validate_specified_datatypes_variables(variables: Dict[str, Any], template:
if not _is_of_exact_type(variable_value, bool):
raise RequestParameterInvalidException(f"Variable value for variable '{name}' must be of type bool")
# Run custom validators if present and value is not None or empty
if template_variable.validators and variable_value is not None and variable_value != "":
# Run custom validators if present.
if template_variable.validators:
for validator in template_variable.validators:
_run_variable_validator(validator, variable_value, name)
test/unit/util/test_config_template_validation.py
+105 -92
View File
@@ -346,103 +346,34 @@ def test_range_validator():
assert isinstance(e, RequestParameterInvalidException)
def test_validators_skip_empty_optional_values():
"""Test that validators skip empty optional values."""
validator = LengthParameterValidatorModel(
min=3,
message="Value must be at least 3 characters",
)
template = _template_with_variable(
TemplateVariableString(name="optional_field", help=None, type="string", default="", validators=[validator])
)
def test_variable_default_does_not_imply_optional():
"""A default value (including empty string) must not make a variable optional."""
template = _template_with_variable(TemplateVariableString(name="string_var", help=None, type="string", default=""))
# Empty value should not trigger validator
instance = _test_instance_with_variables({"optional_field": ""})
validate_secrets_and_variables(instance, template)
def test_validators_skip_null_values():
"""Test that validators skip empty values for optional fields with defaults."""
validator = LengthParameterValidatorModel(
min=3,
message="Value must be at least 3 characters",
)
template = _template_with_variable(
TemplateVariableString(name="optional_field", help=None, type="string", default="", validators=[validator])
)
# Empty value should not trigger validator when field has empty default
instance = _test_instance_with_variables({})
validate_secrets_and_variables(instance, template)
def test_secret_with_default_is_optional():
"""Test that secrets with default values are optional."""
secret = TemplateSecret(name="mysecret", help="Help for secret.", default="default_value")
template = TestTemplate(
id=TEST_TEMPLATE_ID,
version=TEST_TEMPLATE_VERSION,
variables=[],
secrets=[secret],
environment=None,
)
# Should not require the secret when it has a default
instance = _test_instance_with_secrets({})
validate_secrets_and_variables(instance, template)
def test_validators_run_on_optional_fields_with_values():
"""Test that validators run on optional fields when they have values."""
validator = LengthParameterValidatorModel(
min=3,
message="Value must be at least 3 characters",
)
template = _template_with_variable(
TemplateVariableString(name="optional_field", help=None, type="string", default="ok", validators=[validator])
)
# Should run validator when optional field has a value
instance = _test_instance_with_variables({"optional_field": "ab"}) # Too short
e = assert_validation_throws(instance, template)
assert isinstance(e, RequestParameterInvalidException)
assert "at least 3 characters" in str(e)
# Should pass validation with valid value
instance = _test_instance_with_variables({"optional_field": "valid"})
validate_secrets_and_variables(instance, template)
def test_variable_with_null_default():
"""Test variables with null defaults are still required (None is not a valid default)."""
template = _template_with_variable(
TemplateVariableString(name="optional_var", help=None, type="string", default=None)
)
# Should still require the variable when default is None (None is not a valid default)
# Default alone does not make the variable optional
instance = _test_instance_with_variables({})
e = assert_validation_throws(instance, template)
assert isinstance(e, RequestParameterMissingException)
def test_variable_optional_flag_allows_omission_without_default():
"""optional=True alone allows a variable to be omitted even without a default."""
template = _template_with_variable(
TemplateVariableString(name="optional_var", help=None, type="string", optional=True)
)
# Should not require the variable when optional=True
instance = _test_instance_with_variables({})
validate_secrets_and_variables(instance, template)
# Should pass when variable is provided
instance = _test_instance_with_variables({"optional_var": "some_value"})
validate_secrets_and_variables(instance, template)
def test_variable_with_empty_string_default():
"""Test variables with empty string defaults are optional."""
template = _template_with_variable(
TemplateVariableString(name="optional_var", help=None, type="string", default="")
)
# Should not require the variable when it has an empty string default
instance = _test_instance_with_variables({})
validate_secrets_and_variables(instance, template)
def test_secret_without_default_is_required():
"""Test that secrets without defaults are required."""
secret = TemplateSecret(name="requiredsecret", help="Help for secret.")
def test_secret_optional_flag_allows_omission_without_default():
"""optional=True alone allows a secret to be omitted even without a default."""
secret = TemplateSecret(name="optional_secret", help="Help for secret.", optional=True)
template = TestTemplate(
id=TEST_TEMPLATE_ID,
version=TEST_TEMPLATE_VERSION,
@@ -451,12 +382,94 @@ def test_secret_without_default_is_required():
environment=None,
)
# Should require the secret when it has no default
# Should not require the secret when optional=True
instance = _test_instance_with_secrets({})
e = assert_validation_throws(instance, template)
assert isinstance(e, RequestParameterMissingException)
assert "requiredsecret" in str(e)
validate_secrets_and_variables(instance, template)
# Should pass when secret is provided
instance = _test_instance_with_secrets({"requiredsecret": "myvalue"})
instance = _test_instance_with_secrets({"optional_secret": "myvalue"})
validate_secrets_and_variables(instance, template)
def test_optional_variable_validators_run_only_when_value_is_provided():
"""Optional variables are not required, but validators run when a value is supplied."""
validator = LengthParameterValidatorModel(
min=3,
message="Value must be at least 3 characters",
)
template = _template_with_variable(
TemplateVariableString(name="optional_var", help=None, type="string", optional=True, validators=[validator])
)
# Should not require the variable when optional=True
instance = _test_instance_with_variables({})
validate_secrets_and_variables(instance, template)
# Should run validator when value is provided but invalid
instance = _test_instance_with_variables({"optional_var": "ab"}) # Too short
e = assert_validation_throws(instance, template)
assert isinstance(e, RequestParameterInvalidException)
assert "at least 3 characters" in str(e)
# Should pass with valid value
instance = _test_instance_with_variables({"optional_var": "valid"})
validate_secrets_and_variables(instance, template)
# Defaults for optional fields are validated when they are applied
# Invalid default should fail validation
template_with_bad_default = _template_with_variable(
TemplateVariableString(
name="optional_var",
help=None,
type="string",
optional=True,
default="ab", # too short
validators=[validator],
)
)
instance = _test_instance_with_variables({})
e = assert_validation_throws(instance, template_with_bad_default)
assert isinstance(e, RequestParameterInvalidException)
# Valid default should pass validation
template_with_good_default = _template_with_variable(
TemplateVariableString(
name="optional_var",
help=None,
type="string",
optional=True,
default="good",
validators=[validator],
)
)
instance = _test_instance_with_variables({})
validate_secrets_and_variables(instance, template_with_good_default)
def test_variable_optional_with_valid_default():
"""Test that optional variables with valid defaults work correctly."""
# Optional string variable with default
template = _template_with_variable(
TemplateVariableString(name="optional_string", help=None, type="string", optional=True, default="default_value")
)
# Should use default when not provided
instance = _test_instance_with_variables({})
validate_secrets_and_variables(instance, template)
# Should accept override
instance = _test_instance_with_variables({"optional_string": "override"})
validate_secrets_and_variables(instance, template)
# Optional integer variable with default
template = _template_with_variable(
TemplateVariableInteger(name="optional_int", help=None, type="integer", optional=True, default=100)
)
instance = _test_instance_with_variables({})
validate_secrets_and_variables(instance, template)
# Optional boolean variable with default
template = _template_with_variable(
TemplateVariableBoolean(name="optional_bool", help=None, type="boolean", optional=True, default=False)
)
instance = _test_instance_with_variables({})
validate_secrets_and_variables(instance, template)