[BUG] - 2024.9.1 Immutable Field Check Appears to Fail for Certain Schema Types

kenafoster commented 1 month ago

Describe the bug

I'm running into an odd error with the nebari-self-registration extension and changing its config values when core nebari is 2024.9.1 Starting with a config block like the one below, I am unable to change the value of the self_registration.values.images.tag field self_registration: namespace: self-registration

  values:
    image:
      repository: quay.io/nebari/nebari-self-registration
      tag: "20240926-1841"
      #tag: "20241011-1937"

The end of the trace for the error is:

│.../_nebari/stages/terr │
│ aform_state/__init__.py:279 in check_immutable_fields                                            │
│                                                                                                  │
│   276 │   │   │   │   │   │   else:                                                              │
│   277 │   │   │   │   │   │   │   raise e                                                        │
│   278 │   │   │   extra_field_schema = schema.ExtraFieldSchema(                                  │
│ ❱ 279 │   │   │   │   **bottom_level_schema.model_fields[keys[-1]].json_schema_extra or {}       │
│   280 │   │   │   )                                                                              │
│   281 │   │   │   if extra_field_schema.immutable:                                               │
│   282 │   │   │   │   key_path = ".".join(keys)                                                  │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
AttributeError: 'CommentedMap' object has no attribute 'model_fields'

It seems to me that the check_immutable_fields() runs into a problem specifically because the values key schema definition is values: Optional[Dict[str, Any]] = {} ( this is because the whole block is just passed to helm chart values so it would be a headache to define the whole thing explicilty)

I'm unsure about how all of the Pydantic + pluggy schema building magic works, but this is my conclusion since changing fields that are key-value pairs (for example self_registration.namespace ) doesn't encounter this problem

I think there are other values fields in the core Nebari schema that will have the same problem this extension is having. I'll try and recreate the issue with one of those and comment here.

Expected behavior

Changing the values of anything under the values key should be allowed

OS and architecture in which you are running Nebari

MacOS Sequoia (Apple Silicon ARM)

How to Reproduce the problem?

Deploy Nebari 2024.9.1 with the nebari-self-registration extension plugin installed and config in the values block like in the description above

Try to change any of the content in self_registration.values and redeploy

Command output

AttributeError: 'CommentedMap' object has no attribute 'model_fields'

Versions and dependencies used.

Nebari 2024.9.1 nebari-plugin-self-registration 0.0.9

Python 3.12.5

Compute environment

None

Integrations

No response

Anything else?

No response

Adam-D-Lewis commented 1 month ago

We are assuming that all the parts of the Nebari config file are a pydantic model, but in this case it is not. I agree that there are probably other places in Nebari core codeabse where this will be an issue. E.g. I think any of the overrides keys would cause this issue.

If you want to declare some part of the schema as immutable, then it would need to be a pydantic model (as we currently have it), but I think that's a reasonable restriction. We could loosen the restriction of requiring pydantic models or alternatively you could make the values key a blank pydantic model which allows extra fields (e.g. https://docs.pydantic.dev/latest/concepts/models/#extra-fields).

Adam-D-Lewis commented 1 month ago

I think a solution would just change the part of the code erroring out to

            if isinstance(bottom_level_schema, BaseModel):                
                extra_field_schema = schema.ExtraFieldSchema(
                    **bottom_level_schema.model_fields[keys[-1]].json_schema_extra or {}
                )
            else:
                extra_field_schema = schema.ExtraFieldSchema()

or similar and add a test to nebari/tests/tests_unit/test_stages.py showing that this fixes the issue.

@kenafoster Do you have interest in contributing a fix for this?

kenafoster commented 1 month ago

@Adam-D-Lewis yep - happy to work on it - assigned to myself!

marcelovilla commented 1 month ago

@kenafoster we've added this issue to the 2024.9.2 hotfix release milestone: https://github.com/nebari-dev/nebari/milestone/52

nebari-dev / nebari