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

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

Open
kenafoster opened this issue Oct 12, 2024 · 3 comments
Assignees

Comments

@kenafoster
Copy link
Contributor

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

@kenafoster kenafoster added type: bug 🐛 Something isn't working needs: triage 🚦 Someone needs to have a look at this issue and triage labels Oct 12, 2024
@kenafoster kenafoster changed the title [BUG] - 2024.9.1 [BUG] - 2024.9.1 Immutable Field Check Appears to Fail for Certain Schema Types Oct 12, 2024
@marcelovilla marcelovilla added area: user experience 👩🏻‍💻 and removed needs: triage 🚦 Someone needs to have a look at this issue and triage labels Oct 14, 2024
@Adam-D-Lewis
Copy link
Member

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
Copy link
Member

Adam-D-Lewis commented Oct 14, 2024

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 kenafoster self-assigned this Oct 14, 2024
@kenafoster
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: New 🚦
Development

No branches or pull requests

3 participants