Fix: fixed the use of Pydantic for validation of env config

[sushant-suse]

Related Issue: #21

Added new files , modified files

1. src/docbuild/models/config_model/env.py

• Changed Made: Defines the complete EnvConfig Pydantic hierarchy (Env_Server, Env_PathsConfig, etc.).
• Why Changed: This file creates the definitive schema for env.toml. It enforces strict type checking (e.g., ensuring paths are Path objects and hostnames are valid domains/IPs) and includes the @model_validator(mode='before') hook to process and resolve all placeholders ({...}) in the raw dictionary before Pydantic begins validation.

2. src/docbuild/models/config_model/__init__.py

• Changed Made: Added export statements for the new environment model.
• Why Changed: Exposes the new model (EnvConfig) to the application package, allowing other modules (like cmd_cli.py) to import it using relative paths (from ..models.config_model.env import EnvConfig).

3. tests/models/config_model/__init__.py

• Changed Made: Standard Python packaging requirement for the test directory structure.
• Why Changed: Ensures the test structure mirrors the source structure, allowing test frameworks to correctly discover unit tests within the tests/models/config_model directory.

4. src/docbuild/cli/cmd_cli.py

• Changed Made: Updated Phase 2 (Environment Config Load) to import and use EnvConfig.from_dict(raw_env_data). The manual call to replace_placeholders(...) was removed.
• Why Changed: This completes the integration. The raw environment data is now validated against the new schema, and the validated Pydantic object replaces the raw dictionary in the context. The removal of the manual placeholder call shifts that responsibility entirely to the Pydantic model's model_validator hook.

5. src/docbuild/config/load.py

• Changed Made: Removed the obsolete function process_envconfig() and its associated logic.
• Why Changed :The logic previously housed in process_envconfig (file searching and raw loading) is now fully handled by the generic handle_config function and the subsequent Pydantic validation in the CLI. Removing the dedicated, now-redundant function cleans up the core library.

6. tests/cli/cmd_config/test_config.py

• Changed Made:: Modified the mock_full_cli_setup fixture to provide two sequential return values for handle_config (AppConfig data first, then EnvConfig data).
• Why Changed: This sequence mock is crucial to prevent the CLI from crashing (SystemExit(1)) during initialization, as the main cli() function calls the config loader twice in a mandatory order.

7. tests/cli/test_cmd_cli.py

• Changed Made: Updated the primary mock_config_models fixture to ensure mock AppConfig instances correctly expose the .logging.model_dump method (to satisfy logging setup). Also, updated test assertions to expect Pydantic objects instead of raw dictionaries.
• Why Changed: Guarantees that the integration tests correctly verify the full application lifecycle, from raw data loading to validated object storage in the DocBuildContext.

8. tests/config/test_load.py

• Changed Made: Deleted all tests that called the now-removed process_envconfig function.
• Why Changed: This resolves immediate test failures and ensures the file only tests the generic handle_config utility, aligning the test file with the simplified source code (src/docbuild/config/load.py).

9. tests/conftest.py

• Changed Made: Removed obsolete fixtures (like fake_envfile) that were trying to patch the deleted function process_envconfig.
• Why Changed: Removes residual dependency patching that would otherwise cause an AttributeError during test collection and setup.

10. tests/cli/cmd_config/test_environment.py

• Changed Made: Deleted the file.
• Why Changed: This test file was causing a fatal AttributeError during test setup because its fixtures relied on the obsolete process_envconfig function. Its removal stabilizes the overall test suite.

[tomschr]

Thank you for all your efforts! 👍 I was a bit more thorough, so it took longer. 😉 Don't be too much scared, most of them are tiny examples.

A good way would to check if everything is okay is to run this command:

docbuild --env-config=etc/docbuild/env.example.toml config env

At the moment I get 34 Pydantic errors. Not sure if I understand all of them.

If everything okay it should print the resolved config as JSON output.

[tomschr]

Thanks! I have some smaller comments. 🙂

[tomschr]

Found a minor thing that I forgot, sorry.

Regarding the coverage, I see:

src/docbuild/models/path.py                       35      3      8      1  90.7%   30, 64-65

Line 30 will be automatically gone if you apply the Path(path).expanduser().resolve() fix. Could you check the other lines and cover them? It's a check whether the path exists and the test should catch the OSError.

[tomschr]

Oh, I noticed something in the error message. For example:

paths.tmp.tmp_base_path
  Field required [type=missing, input_value={'tmp_base_dir': '/var/tm...{docset}_{lang}_XXXXXX'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.11/v/missing

That's certainly wrong as the path contains placeholders. These are resolved during execution of the code. So I guess they can't be checked. As such, we probably need to use a different type (probably just Path?) We need to decide what we use here.

I also noticed this one:

paths.tmp.tmp_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/var/tmp/docbuild/doc-example-com', input_type=str]

I'm not sure about these "extra inputs are not permitted" message. Do you have an idea?

[sushant-suse]

Oh, I noticed something in the error message. For example:

paths.tmp.tmp_base_path
  Field required [type=missing, input_value={'tmp_base_dir': '/var/tm...{docset}_{lang}_XXXXXX'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.11/v/missing

That's certainly wrong as the path contains placeholders. These are resolved during execution of the code. So I guess they can't be checked. As such, we probably need to use a different type (probably just Path?) We need to decide what we use here.

I also noticed this one:

paths.tmp.tmp_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/var/tmp/docbuild/doc-example-com', input_type=str]

I'm not sure about these "extra inputs are not permitted" message. Do you have an idea?

Hi @tomschr, thanks so much for running the full validation tests and sharing these specific errors. This confirms the Pydantic validation is working correctly by strictly enforcing the schema. The errors point to two main issues between the Pydantic model structure and the existing env.example.toml file.

Fix for Placeholder Crash (TMP_PATH not found)

The errors about {TMP_PATH} not being found (which you saw in the previous full error logs) and the one flagged here about the missing field are related to a field naming mismatch in the Env_TmpPaths model.

• The existing TOML file uses the key: tmp_base_dir.
• Your Pydantic model requires the key: tmp_base_path.

Since the names don't match, Pydantic throws an error saying the field tmp_base_path is missing (missing type), and the placeholder resolution (_resolve_placeholders) fails because it tries to look up placeholder values that depend on the correct base key (tmp_base_dir).

To solve this, we can update the Env_TmpPaths class to change the field name to tmp_base_dir to match the existing TOML file structure, resolving this structural conflict and allowing the placeholder dependencies to resolve correctly.

Fix for "Extra inputs are not permitted"

The error - paths.tmp.tmp_dir Extra inputs are not permitted [type=extra_forbidden, input_value=...]. This error means that the section being validated contains a piece of data that is present in the raw input but not declared as a field in the Pydantic schema, due to the ConfigDict(extra='forbid') setting.

The failure is occurring because the existing env.example.toml file contains obsolete sections or attributes. To solve this, we have two options:

1. Merge Option: We can remove all obsolete keys/sections like [build], [build.daps], and any other keys outside of [server], [config], [paths], and [xslt-params] from the env.example.toml file. This brings the input file into compliance with the new, strict schema.
2. Code Option: If the file must contain those sections (e.g., for compatibility), we could change the top-level EnvConfig model to use extra='allow' for the entire model, but this defeats the purpose of validation.

[tomschr]

Ok, so this time I didn't look at the code and just run the config subcommand:

docbuild  --env-config=etc/docbuild/env.example.toml config env

This gave me many validation errors:

Validation errors

[2025-11-26 12:08:12] [ERROR] docbuild.cli.cmd_cli: Environment configuration failed validation: Error in config file(s): (PosixPath('etc/docbuild/env.example.toml'),) 19 validation errors for EnvConfig
paths.tmp.tmp_path
  Field required [type=missing, input_value={'tmp_base_dir': '/var/tm...{docset}_{lang}_XXXXXX'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.12/v/missing
paths.tmp.tmp_deliverable_path
  Field required [type=missing, input_value={'tmp_base_dir': '/var/tm...{docset}_{lang}_XXXXXX'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.12/v/missing
paths.tmp.tmp_build_dir
  Value error, Could not create directory '/var/tmp/docbuild/doc-example-com/build/{product}-{docset}-{lang}': [Errno 13] Permission denied: '/var/tmp/docbuild/doc-example-com/build' [type=value_error, input_value=PosixPath('/var/tmp/docbu...oduct}-{docset}-{lang}'), input_type=PosixPath]
    For further information visit https://errors.pydantic.dev/2.12/v/value_error
paths.tmp.tmp_out_path
  Field required [type=missing, input_value={'tmp_base_dir': '/var/tm...{docset}_{lang}_XXXXXX'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.12/v/missing
paths.tmp.log_path
  Field required [type=missing, input_value={'tmp_base_dir': '/var/tm...{docset}_{lang}_XXXXXX'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.12/v/missing
paths.tmp.tmp_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/var/tmp/docbuild/doc-example-com', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden
paths.tmp.tmp_metadata_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/var/tmp/docbuild/doc-example-com/metadata', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden
paths.tmp.tmp_deliverable_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/var/tmp/docbuild/doc-example-com/deliverable/', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden
paths.tmp.tmp_out_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/var/tmp/docbuild/doc-example-com/out/', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden
paths.tmp.log_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/var/tmp/docbuild/doc-example-com/log/', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden
paths.target.target_path
  Field required [type=missing, input_value={'target_dir': '[email protected]/external-builds/'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.12/v/missing
paths.target.backup_path
  Field required [type=missing, input_value={'target_dir': '[email protected]/external-builds/'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.12/v/missing
paths.target.target_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='[email protected]:/srv/docs', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden
paths.target.backup_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/data/docbuild/external-builds/', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden
paths.root_config_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/etc/docbuild', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden
paths.jinja_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/etc/docbuild/jinja-doc-suse-com', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden
paths.server_rootfiles_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/etc/docbuild/server-root-files-doc-suse-com', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden
paths.base_server_cache_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/var/cache/docserv/doc-example-com', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden
paths.base_tmp_dir
  Extra inputs are not permitted [type=extra_forbidden, input_value='/var/tmp/docbuild', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/extra_forbidden

Perhaps I should probably explain it better. 🙂 Ok, here is the thing. Inside the ENV config, we have two kind of potential placeholders:

• Regular/config placeholders Syntax {placeholder}
  They are used to reference other parts of the ENV configuration. For example, if you have "{tmp_base_dir}/doc-example-com" as string, the placeholder tmp_base_dir is searched in the current section of the config.
  It's possible to refer to other sections of the config with section.key like in paths.base_tmp_dir.
  Whatever variation we use, at the end it should resolve to at least some path that can be accessed.

• Runtime placeholders Syntax {{placeholder}}
  These types of placeholders cannot and must not be replaced. They are replaced at runtime with specific values.

Unfortunately, the tricky part is, that sometimes we have both types in our TOML:

tmp_build_dir = "{tmp_dir}/build/{{product}}-{{docset}}-{{lang}}" 

You can only resolve the first placeholders, but not the others. Trying to create that directory will fail (as seen above in the error messages).

I would suggest to discuss the following possible solutions to mitigate this:

1. Maybe we introduce a better Pydantic type? A type that can deal with these placeholders?
   For examples, it could hold the resolved part and if it still sees placeholders in it, it doesn't try to resolve it. Maybe it can try to resolve some parts. In the example above, we could try to resolve {tmp_dir} placeholder and see if it has the right privileges. But we can't do more.
   Maybe we need to rewrite and rename EnsureWritableDirectory to something more appropriate?

2. Add specific unit tests.
   You added tests/models/config_model/test_env.py which is fine, but maybe... a bit too general. All of your tests use EnvConfig. Perhaps it would be easier if you test our individual models: a test for Env_BuildDaps, another for Env_BuildContainer etc.
   That would make the tests shorter as you don't have to deal with all sorts of secondary problems. Additionally, you would find problems faster as the test shows exactly where it failed.
   One way to organize it could be to create a tests/models/config_model/env/ directory and add individual files (like test_env_builddaps.py, test_env_buildcontainer.py etc.) for each of the models.
   Maybe rename tests/models/config_model/test_env.py to tests/models/config_model/test_envconfig.py to make it more obvious? But that's just an idea, not really needed.

3. Maybe we should disallow the mixture of regular and runtime placeholders in TOML config?
   I guess we can get rid of dealing with another type like in item 1. That would be a way to make it cleaner and more obvious. We could separate the variable part from the constant part like this:

   tmp_build_dir = "{tmp_dir}/build/"                # <= maybe type EnsureWritableDirectory?
   build_fragment="{{product}}-{{docset}}-{{lang}}"  # <= type str

   Of course, the code that depends on tmp_build_dir and expects the runtime placeholders would need to be changed.

Another thing that caught my eye:

• We have src/docbuild/models/config_model. I think, that's a misnomer as we have "models" already in the parent directory. Why not just config? Would be shorter and easier to type. 😉

Ok, thanks!