Skip to content

Commit

Permalink
Prevent undefined environment variable as LSF_SERVER value
Browse files Browse the repository at this point in the history
  • Loading branch information
larsevj committed Sep 26, 2023
1 parent d5b8101 commit 96d805f
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 9 deletions.
15 changes: 7 additions & 8 deletions src/ert/config/parsing/config_schema_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,10 @@ def token_to_value_with_context(

if not self._is_in_allowed_values_for_arg_at_index(token, index):
raise ConfigValidationError.with_context(
f"{self.kw!r} argument {index + 1!r} must be one of"
f" {self.indexed_selection_set[index]!r} was {token.value!r}",
(
f"{self.kw!r} argument {index + 1!r} must be one of"
f" {self.indexed_selection_set[index]!r} was {token.value!r}"
),
token,
)

Expand All @@ -104,17 +106,15 @@ def token_to_value_with_context(
return ContextBool(False, token, keyword)
else:
raise ConfigValidationError.with_context(
f"{self.kw!r} must have a boolean value"
f" as argument {index + 1!r}",
f"{self.kw!r} must have a boolean value as argument {index + 1!r}",
token,
)
if val_type == SchemaItemType.INT:
try:
return ContextInt(int(token), token, keyword)
except ValueError as e:
raise ConfigValidationError.with_context(
f"{self.kw!r} must have an integer value"
f" as argument {index + 1!r}",
f"{self.kw!r} must have an integer value as argument {index + 1!r}",
token,
) from e
if val_type == SchemaItemType.FLOAT:
Expand Down Expand Up @@ -161,8 +161,7 @@ def token_to_value_with_context(

if os.path.isdir(absolute_path):
raise ConfigValidationError.with_context(
f"Expected executable file, "
f"but {token.value!r} is a directory.",
f"Expected executable file, but {token.value!r} is a directory.",
token,
)

Expand Down
8 changes: 8 additions & 0 deletions src/ert/config/queue_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ def from_dict(cls, config_dict: ConfigDict) -> QueueConfig:
f"Valid choices are {sorted(VALID_QUEUE_OPTIONS[queue_system])}."
)
if values:
if option_name == "LSF_SERVER" and values[0].startswith("$"):
raise ConfigValidationError(
"Invalid server name specified for QUEUE_OPTION LSF"
f" LSF_SERVER: {values[0]}. Server name is currently an"
" undefined environment variable. The LSF_SERVER keyword is"
" usually provided by the site-configuration file, beware that"
" you are effectively replacing the default value provided."
)
queue_options[queue_system].append((option_name, values[0]))
else:
queue_options[queue_system].append(option_name)
Expand Down
27 changes: 26 additions & 1 deletion tests/unit_tests/config/test_queue_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@
import pytest
from hypothesis import given

from ert.config import ConfigValidationError, ErtConfig, QueueConfig, QueueSystem
from ert.config import (
ConfigValidationError,
ErtConfig,
QueueConfig,
QueueSystem,
)
from ert.job_queue import Driver


Expand Down Expand Up @@ -160,3 +165,23 @@ def test_overwriting_QUEUE_OPTIONS_warning(
f"Overwriting QUEUE_OPTION {queue_system} {queue_system_option}: \n Old value:"
" test_0 \n New value: test_1" in caplog.text
)


@pytest.mark.usefixtures("use_tmpdir", "set_site_config")
def test_undefined_LSF_SERVER_environment_variable():
filename = "config.ert"
with open(filename, "w", encoding="utf-8") as f:
f.write("NUM_REALIZATIONS 1\n")
f.write("QUEUE_SYSTEM LSF\n")
f.write("QUEUE_OPTION LSF LSF_SERVER $MY_SERVER\n")
with pytest.raises(
ConfigValidationError,
match=(
r"Invalid server name specified for QUEUE_OPTION LSF LSF_SERVER: "
r"\$MY_SERVER. Server name is currently an undefined environment variable."
r" The LSF_SERVER keyword is usually provided by the site-configuration"
r" file, beware that you are effectively replacing the default value"
r" provided."
),
):
ErtConfig.from_file(filename)

0 comments on commit 96d805f

Please sign in to comment.