Skip to content

feat(node): return bare IP addresses for attributes declaring allow_prefix [INFP-551] - #1220

Open
ajtmccarty wants to merge 4 commits into
infrahub-developfrom
infp-551-bare-ip-attribute
Open

feat(node): return bare IP addresses for attributes declaring allow_prefix [INFP-551]#1220
ajtmccarty wants to merge 4 commits into
infrahub-developfrom
infp-551-bare-ip-attribute

Conversation

@ajtmccarty

@ajtmccarty ajtmccarty commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Counterpart to opsmill/infrahub#10081, the first PR in the Infrahub-side stack for INFP-551.

Why

Infrahub is gaining a parameters.allow_prefix declaration on IPHost attributes. When it is false, the
attribute holds a bare IP address — 10.0.0.1 rather than the 10.0.0.1/32 the server previously
invented — and a real subnet prefix is refused.

Without this change the SDK re-attaches the mask the server just removed: every IPHost response is parsed
with ipaddress.ip_interface, so a bare 10.0.0.1 becomes IPv4Interface('10.0.0.1/32') and the mask
reappears at the SDK boundary. That directly contradicts the feature's contract, which is why this ships
alongside the server change rather than after it.

What changed

  • Value coercion (node/attribute.py): for IPHost, select ipaddress.ip_address when allow_prefix
    is False and ipaddress.ip_interface otherwise. The flag is read tolerantly as
    (self._schema.parameters or {}).get("allow_prefix", True), because AttributeSchemaAPI.parameters is
    dict[str, Any] | None — an SDK talking to an older server that does not send the key behaves exactly as
    it does today.
  • Removed the dead "IPAddress" kind key. No such attribute kind exists server-side: the generated
    AttributeKind enum has no IPADDRESS member, and two tests exercising it were already skipped citing
    that. The ip_address callable it referenced becomes the declared-IPHost branch.
  • Protocol generation (protocols_generator/generator.py): emit IPAddress / IPAddressOptional for a
    declared attribute, IPHost / IPHostOptional otherwise, composing with the existing
    optional and default_value is None suffix rule. All four protocol names were already registered and
    imported, so no change was needed to constants.py, protocols_base.py, or template.j2.
  • Regenerated schema/generated/{read,write}.py, which gain IPHostAttributeParameters{Read,Write} and
    IPHostAttribute{Read,Write}, with AttributeKind.IPHOST moving out of the generic variant.
  • Documented the version floor in the compatibility matrix.

Why the generated models matter to the server

More than a type nicety: SchemaLoadAPI on the server inherits these write models. Without
IPHostAttributeWrite, an IPHost attribute matches GenericAttributeWrite, whose parameters is
field-less with extra="ignore" — so allow_prefix is silently dropped from every schema-load payload
and the server-side feature is inert. The Infrahub PR cannot pass CI until this branch is available.

How to review

node/attribute.py and protocols_generator/generator.py are the whole change; everything else is
generated output or tests.

Coverage follows a three-way pattern rather than two: declared, undeclared, and schema payload
with no allow_prefix key at all
— the last being the old-server tolerance case, which is the one most
likely to regress silently.

  • test_node_IPHost_deserialization_honours_allow_prefix — 7 cases × 2 client types
  • test_filter_render_iphost_attribute — 8 cases
  • Two previously-skipped IPAddress tests repurposed onto declared IPHost rather than deleted

Merge order

This should merge before opsmill/infrahub#10081. That PR pins this branch's commit provisionally so
both can be reviewed in parallel; once this merges, the Infrahub submodule pointer is re-pinned to the
merged commit and confirmed an ancestor of infrahub-develop.


Summary by cubic

Return bare IP addresses for IPHost attributes that set parameters.allow_prefix: false, and mark them as IPAddress in generated protocols. Addresses INFP-551 by delivering bare IPs without a new attribute kind, while keeping old-server behavior.

  • New Features

    • Coerce IPHost with ipaddress.ip_address when allow_prefix is false; otherwise use ipaddress.ip_interface.
    • Generate IPAddress/IPAddressOptional types for flagged IPHost; otherwise keep IPHost/IPHostOptional.
    • Add IPHostAttribute{Read,Write} with parameters.allow_prefix (regenerated after description tweak) and update compatibility docs with min versions (SDK ≥ 1.23.0, Infrahub ≥ 1.11).
  • Migration

    • Behavior change: flagged attributes return IPv4Address/IPv6Address (no mask) instead of interfaces.
    • Compatibility: older servers are safe; when allow_prefix is absent, values remain interfaces. The server must be ≥ 1.11 to publish the flag.

Written for commit 525b28f. Summary will update on new commits.

Review in cubic

…refix [INFP-551]

An IPHost attribute whose schema sets parameters.allow_prefix to false stores a
bare address, so parsing it as an interface re-attached the host mask at the SDK
boundary. Value coercion and the generated-protocol annotation now read the flag,
tolerantly defaulting to the prefixed behaviour when a server does not publish it.

Removes the unreachable IPAddress attribute-kind branch: no such attribute kind
exists on the server, and the bare-address coercion it referenced is what the
flagged IPHost branch now uses.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the type/documentation Improvements or additions to documentation label Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

❗ There is a different number of reports uploaded between BASE (973eaa9) and HEAD (525b28f). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (973eaa9) HEAD (525b28f)
integration-tests 1 0
@@                 Coverage Diff                  @@
##           infrahub-develop    #1220      +/-   ##
====================================================
- Coverage             83.69%   77.48%   -6.22%     
====================================================
  Files                   144      144              
  Lines                 12754    12776      +22     
  Branches               1859     1863       +4     
====================================================
- Hits                  10674     9899     -775     
- Misses                 1516     2326     +810     
+ Partials                564      551      -13     
Flag Coverage Δ
integration-tests ?
python-3.10 55.95% <42.30%> (-0.01%) ⬇️
python-3.11 55.94% <42.30%> (-0.02%) ⬇️
python-3.12 55.95% <42.30%> (-0.02%) ⬇️
python-3.13 55.95% <42.30%> (-0.02%) ⬇️
python-3.14 55.93% <42.30%> (-0.02%) ⬇️
python-filler-3.12 24.13% <57.69%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/node/attribute.py 100.00% <100.00%> (+1.85%) ⬆️
infrahub_sdk/protocols_generator/generator.py 95.29% <100.00%> (+0.17%) ⬆️
infrahub_sdk/schema/generated/read.py 100.00% <100.00%> (ø)
infrahub_sdk/schema/generated/write.py 99.40% <100.00%> (+0.02%) ⬆️

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 12 files

Re-trigger cubic

ajtmccarty and others added 2 commits July 29, 2026 17:20
…g a coercer

_value_coercer returned a callable that was invoked immediately at its only
call site, so the indirection bought nothing. Coerce in place instead, and drop
the now-unused Callable import.

Also trims the parameter comments to a single line.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vale rejects 'onwards'. The warning admonition below already says 'or later',
so this matches it.

Edited in the Jinja2 template, since compatibility.mdx is generated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 30, 2026

Copy link
Copy Markdown

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 525b28f
Status: ✅  Deploy successful!
Preview URL: https://64e0058c.infrahub-sdk-python.pages.dev
Branch Preview URL: https://infp-551-bare-ip-attribute.infrahub-sdk-python.pages.dev

View logs

@ajtmccarty
ajtmccarty marked this pull request as ready for review July 30, 2026 01:20
@ajtmccarty
ajtmccarty requested a review from a team as a code owner July 30, 2026 01:20
…ption

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant