feat(schema): use netplan python api for schema validation annotation #4767

blackboxsw · 2024-01-11T06:31:49Z

Leverage netplan API to perform schema validation when on a system with python3-netplan package.

feat(schema): use netplan API to validate network-config schema
    
    Validation of network-config version: 2 will be performed on systems
    with python3-netplan installed which delivers a python API
    netplan.Parser which allows cloud-init to parse and determine specific
    netplan schema errors.
    
    The netplan API is only present on Ubuntu Noble in the python3-netplan
    deb package.
    
    Add netplan_validate_network_config function to use netplan API when
    present. Wire this function into runtime schema validation at initial
    boot to warn about network-config invalid version: 2 schema.
    
    Also wire this function into cloud-init schema command to warn and
    annotate specific schema errors by line in datasource provided
    network-config.

Context:

unittests are challenging and heavily mocked because the netplan project https://github.com/canonical/netplan/ is not easily consumable as test dependency for tox as pip cannot cleanly determine that this is a python project and netplan is not published to pypi. There is significant mocking in unit tests yet integration tests cover netplan-based schema validation of network version: 2 config.

Checklist before ready for review:

unit test coverage for new schema functionality
integration testing for netplan-based schema validation of version: 2 network config

Additional Context

Test Steps

CLOUD_INIT_CLOUD_INIT_SOURCE=IN_PLACE CLOUD_INIT_OS_IMAGE=noble tox -e integration-tests -- tests/integration_tests/cmd/test_schema.py
CLOUD_INIT_CLOUD_INIT_SOURCE=IN_PLACE CLOUD_INIT_OS_IMAGE=lunar tox -e integration-tests --  tests/integration_tests/cmd/test_schema.py

# Also tests/integration_tests/test_networking.py

Checklist

My code follows the process laid out in the documentation
I have updated or added any unit tests accordingly
I have updated or added any documentation accordingly

Merge type

Squash merge using "Proposed Commit Message"
Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

TheRealFalcon

Overall nice add. I didn't really look at the tests since you're still working those though I'd suggest having a test that includes a deprecation warning, just to ensure we're handling it correctly.

I left some inline comments (mostly nits). Additionally, a few strings/comments mention python3-netplan, but that is an ubuntu-specific package name. It's really just the netplan API doing the validation, so I think it'd be better to just refer to netplan rather than python3-netplan.

cloudinit/config/schema.py

TheRealFalcon · 2024-01-11T20:14:37Z

cloudinit/config/schema.py

+    # network-config it generates, so create a <tmp_dir>/etc/netplan
+    # to validate only our network-config.
+    parse_dir = mkdtemp()
+    netplan_file = os.path.join(parse_dir, "etc/netplan/network-config.yaml")


Nit: Is the etc/netplan/ prefix necessary?

Unfortunately, yes it is. netplan Parser.load_yaml_hierarchy only looks under the subdir path etc/netplan/ to process yaml parts. Otherwise it happily ignores files in the root subdir of a path

TheRealFalcon · 2024-01-11T20:28:55Z

cloudinit/config/schema.py

@@ -1002,7 +1098,14 @@ def validate_cloudconfig_file(
        if not netcfg:
            print("Skipping network-config schema validation on empty config.")
            return False
-        elif netcfg.get("version") != 1:
+        elif netcfg.get("version") == 2:


If we're validating userdata or network config v1 we call "validate_cloudconfig_schema", but for network config v2 call "netplan_validate_network_config". I think we could a little refactoring as this comes out a little confusing.

I think it's also a little odd that the caller has to check the version number and determine which thing to call. Ideally, we'd have something like "validate_schema" that we pass our configuration to along with some relevant options, and based on what it is, it calls the right validation function.

Good point I refactored netplan_validate_network_config to accept just the network_config dict instead of dealing with files. This simplified callsites sigificantly and allowed dealing with version 2 vs version 1 vs netplan API availability inside netplan_validate_network_schema function. If unable to perform netplan_validate.... we fallthrough and use cloud-init's schema for validation if present. Since we don't have JSON schema version 2 yet, we will still emit Skipping schema validation messages when not netplan and not network schema version: 1.

+1 I was avoiding the refactor to validate_cloudconfig_schema that would require adding a config path. But, I've since refactored this code to simplify call-sites and like cloudinit.stages

) Validation of network-config version: 2 will be performed on systems with python3-netplan installed which delivers a python API netplan.Parser which allows cloud-init to parse and determine specific netplan schema errors. The netplan API is only present on Ubuntu Noble in the python3-netplan deb package. Add netplan_validate_network_config function to use netplan API when present. Wire this function into runtime schema validation at initial boot to warn about network-config invalid version: 2 schema. Also wire this function into cloud-init schema command to warn and annotate specific schema errors by line in datasource provided network-config.

) On Noble, python netplan bindings are available in python3-netplan deb package. The presence of they python bindings and netplan.Parser import allow cloud-int for parse and validate network-config version 2 schema. Add an integration tests asserting both schema validation and annotation of invalid network configuration on Ubuntu Noble. On non-Noble environments, expect Skipping of network-config version: 2 message.

TheRealFalcon

Just some minor issues left inline.

Did you miss this comment from my last review?

Additionally, a few strings/comments mention python3-netplan, but that is an ubuntu-specific package name. It's really just the netplan API doing the validation, so I think it'd be better to just refer to netplan rather than python3-netplan.

cloudinit/config/schema.py

tests/integration_tests/test_networking.py

tests/integration_tests/cmd/test_schema.py

…cal#4767)

) Validation of network-config version: 2 will be performed on systems with python3-netplan installed which delivers a python API netplan.Parser which allows cloud-init to parse and determine specific netplan schema errors. The netplan API is only present on Ubuntu Noble in the python3-netplan deb package. Add netplan_validate_network_config function to use netplan API when present. Wire this function into runtime schema validation at initial boot to warn about network-config invalid version: 2 schema. Also wire this function into cloud-init schema command to warn and annotate specific schema errors by line in datasource provided network-config.

) On Noble, python netplan bindings are available in python3-netplan deb package. The presence of they python bindings and netplan.Parser import allow cloud-int for parse and validate network-config version 2 schema. Add an integration tests asserting both schema validation and annotation of invalid network configuration on Ubuntu Noble. On non-Noble environments, expect Skipping of network-config version: 2 message.

) Validation of network-config version: 2 will be performed on systems with python3-netplan installed which delivers a python API netplan.Parser which allows cloud-init to parse and determine specific netplan schema errors. The netplan API is only present on Ubuntu Noble in the python3-netplan deb package. Add netplan_validate_network_config function to use netplan API when present. Wire this function into runtime schema validation at initial boot to warn about network-config invalid version: 2 schema. Also wire this function into cloud-init schema command to warn and annotate specific schema errors by line in datasource provided network-config.

) On Noble, python netplan bindings are available in python3-netplan deb package. The presence of they python bindings and netplan.Parser import allow cloud-int for parse and validate network-config version 2 schema. Add an integration tests asserting both schema validation and annotation of invalid network configuration on Ubuntu Noble. On non-Noble environments, expect Skipping of network-config version: 2 message.

) Validation of network-config version: 2 will be performed on systems with python3-netplan installed which delivers a python API netplan.Parser which allows cloud-init to parse and determine specific netplan schema errors. The netplan API is only present on Ubuntu Noble in the python3-netplan deb package. Add netplan_validate_network_config function to use netplan API when present. Wire this function into runtime schema validation at initial boot to warn about network-config invalid version: 2 schema. Also wire this function into cloud-init schema command to warn and annotate specific schema errors by line in datasource provided network-config.

) On Noble, python netplan bindings are available in python3-netplan deb package. The presence of they python bindings and netplan.Parser import allow cloud-int for parse and validate network-config version 2 schema. Add an integration tests asserting both schema validation and annotation of invalid network configuration on Ubuntu Noble. On non-Noble environments, expect Skipping of network-config version: 2 message.

TheRealFalcon

LGTM! I'll leave to you to merge

Validation of network-config version: 2 will be performed on systems with python3-netplan installed which delivers a python API netplan.Parser which allows cloud-init to parse and determine specific netplan schema errors. The netplan API is only present on Ubuntu Noble in the python3-netplan deb package. Add netplan_validate_network_config function to use netplan API when present. Wire this function into runtime schema validation at initial boot to warn about network-config invalid version: 2 schema. Also wire this function into cloud-init schema command to warn and annotate specific schema errors by line in datasource provided network-config.

blackboxsw added the wip Work in progress, do not land label Jan 11, 2024

TheRealFalcon self-assigned this Jan 11, 2024

TheRealFalcon reviewed Jan 11, 2024

View reviewed changes

blackboxsw force-pushed the SC-1374-use-netplan-python-api-for-schema-validation-annotation branch 2 times, most recently from ffdadc3 to 6e23c4e Compare January 19, 2024 02:45

blackboxsw removed the wip Work in progress, do not land label Jan 19, 2024

blackboxsw force-pushed the SC-1374-use-netplan-python-api-for-schema-validation-annotation branch 3 times, most recently from 45757c5 to d5ff050 Compare January 19, 2024 04:17

blackboxsw marked this pull request as ready for review January 19, 2024 04:17

blackboxsw force-pushed the SC-1374-use-netplan-python-api-for-schema-validation-annotation branch from d5ff050 to 8897c93 Compare January 19, 2024 04:22

blackboxsw requested a review from TheRealFalcon January 23, 2024 16:17

TheRealFalcon reviewed Jan 24, 2024

View reviewed changes

cloudinit/config/schema.py Outdated Show resolved Hide resolved

tests/integration_tests/test_networking.py Outdated Show resolved Hide resolved

tests/integration_tests/cmd/test_schema.py Outdated Show resolved Hide resolved

blackboxsw added 2 commits January 25, 2024 08:54

fix: cli schema config-file option report network-config type (canoni…

7f6dcd4

…cal#4767)

chore: define CLOUDINIT_NETPLAN_FILE static var (canonical#4767)

a3373ca

blackboxsw force-pushed the SC-1374-use-netplan-python-api-for-schema-validation-annotation branch from 8897c93 to 28f23f3 Compare January 25, 2024 15:58

blackboxsw requested a review from TheRealFalcon January 25, 2024 15:58

blackboxsw force-pushed the SC-1374-use-netplan-python-api-for-schema-validation-annotation branch from 28f23f3 to 8aa0ee0 Compare January 25, 2024 15:59

blackboxsw force-pushed the SC-1374-use-netplan-python-api-for-schema-validation-annotation branch from 8aa0ee0 to 30f05a0 Compare January 25, 2024 16:02

blackboxsw force-pushed the SC-1374-use-netplan-python-api-for-schema-validation-annotation branch from 30f05a0 to ac1f870 Compare January 25, 2024 16:04

TheRealFalcon approved these changes Jan 25, 2024

View reviewed changes

blackboxsw merged commit 39c5781 into canonical:main Jan 26, 2024
29 checks passed

blackboxsw added a commit that referenced this pull request Jan 26, 2024

fix: cli schema config-file option report network-config type (#4767)

81a3fa4

blackboxsw added a commit that referenced this pull request Jan 26, 2024

chore: define CLOUDINIT_NETPLAN_FILE static var (#4767)

dad4ad9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(schema): use netplan python api for schema validation annotation #4767

feat(schema): use netplan python api for schema validation annotation #4767

blackboxsw commented Jan 11, 2024 •

edited

Loading

TheRealFalcon left a comment

TheRealFalcon Jan 11, 2024

blackboxsw Jan 18, 2024

TheRealFalcon Jan 11, 2024

blackboxsw Jan 18, 2024

blackboxsw Jan 19, 2024

TheRealFalcon left a comment

TheRealFalcon left a comment

feat(schema): use netplan python api for schema validation annotation #4767

feat(schema): use netplan python api for schema validation annotation #4767

Conversation

blackboxsw commented Jan 11, 2024 • edited Loading

Additional Context

Test Steps

Checklist

Merge type

TheRealFalcon left a comment

Choose a reason for hiding this comment

TheRealFalcon Jan 11, 2024

Choose a reason for hiding this comment

blackboxsw Jan 18, 2024

Choose a reason for hiding this comment

TheRealFalcon Jan 11, 2024

Choose a reason for hiding this comment

blackboxsw Jan 18, 2024

Choose a reason for hiding this comment

blackboxsw Jan 19, 2024

Choose a reason for hiding this comment

TheRealFalcon left a comment

Choose a reason for hiding this comment

TheRealFalcon left a comment

Choose a reason for hiding this comment

blackboxsw commented Jan 11, 2024 •

edited

Loading