Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bug where check_immutable_fields throws error with old version of Nebari #2796

Merged
merged 3 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 3 additions & 7 deletions src/_nebari/stages/terraform_state/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ def check_immutable_fields(self):

# compute diff of remote/prior and current nebari config
nebari_config_diff = utils.JsonDiff(
nebari_config_state.model_dump(), self.config.model_dump()
nebari_config_state, self.config.model_dump()
)
# check if any changed fields are immutable
for keys, old, new in nebari_config_diff.modified():
Expand All @@ -284,7 +284,7 @@ def check_immutable_fields(self):
f'Attempting to change immutable field "{key_path}" ("{old}"->"{new}") in Nebari config file. Immutable fields cannot be changed after initial deployment.'
)

def get_nebari_config_state(self):
def get_nebari_config_state(self) -> dict:
directory = str(self.output_directory / self.stage_prefix)
tf_state = terraform.show(directory)
nebari_config_state = None
Expand All @@ -294,11 +294,7 @@ def get_nebari_config_state(self):
tf_state.get("values", {}).get("root_module", {}).get("resources", [])
):
if resource["address"] == "terraform_data.nebari_config":
from nebari.plugins import nebari_plugin_manager

nebari_config_state = nebari_plugin_manager.config_schema(
**resource["values"]["input"]
)
nebari_config_state = resource["values"]["input"]
break
return nebari_config_state

Expand Down
22 changes: 17 additions & 5 deletions tests/tests_unit/test_stages.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def terraform_state_stage(mock_config, tmp_path):

@patch.object(TerraformStateStage, "get_nebari_config_state")
def test_check_immutable_fields_no_changes(mock_get_state, terraform_state_stage):
mock_get_state.return_value = terraform_state_stage.config
mock_get_state.return_value = terraform_state_stage.config.model_dump()

# This should not raise an exception
terraform_state_stage.check_immutable_fields()
Expand All @@ -41,7 +41,7 @@ def test_check_immutable_fields_mutable_change(
):
old_config = mock_config.model_copy(deep=True)
old_config.namespace = "old-namespace"
mock_get_state.return_value = old_config
mock_get_state.return_value = old_config.model_dump()

# This should not raise an exception (namespace is mutable)
terraform_state_stage.check_immutable_fields()
Expand All @@ -54,7 +54,7 @@ def test_check_immutable_fields_immutable_change(
):
old_config = mock_config.model_copy(deep=True)
old_config.provider = schema.ProviderEnum.gcp
mock_get_state.return_value = old_config
mock_get_state.return_value = old_config.model_dump()

# Mock the provider field to be immutable
mock_model_fields.__getitem__.return_value.json_schema_extra = {"immutable": True}
Expand All @@ -77,7 +77,7 @@ def test_check_immutable_fields_no_prior_state(mock_get_state, terraform_state_s
def test_check_dict_value_change(mock_get_state, terraform_state_stage, mock_config):
old_config = mock_config.model_copy(deep=True)
terraform_state_stage.config.local.node_selectors["worker"].value += "new_value"
mock_get_state.return_value = old_config
mock_get_state.return_value = old_config.model_dump()

# should not throw an exception
terraform_state_stage.check_immutable_fields()
Expand All @@ -87,7 +87,19 @@ def test_check_dict_value_change(mock_get_state, terraform_state_stage, mock_con
def test_check_list_change(mock_get_state, terraform_state_stage, mock_config):
old_config = mock_config.model_copy(deep=True)
old_config.environments["environment-dask.yaml"].channels.append("defaults")
mock_get_state.return_value = old_config
mock_get_state.return_value = old_config.model_dump()

# should not throw an exception
terraform_state_stage.check_immutable_fields()


@patch.object(TerraformStateStage, "get_nebari_config_state")
def test_check_immutable_fields_old_nebari_version(
mock_get_state, terraform_state_stage, mock_config
):
old_config = mock_config.model_copy(deep=True).model_dump()
old_config["nebari_version"] = "2024.7.1" # Simulate an old version
mock_get_state.return_value = old_config

# This should not raise an exception
terraform_state_stage.check_immutable_fields()
Loading