Skip to content

Add Aruba AOS-CX platform support#289

Open
dspatig-gdx wants to merge 1 commit into
netdevops:masterfrom
dspatig-gdx:aruba-aoscx-support
Open

Add Aruba AOS-CX platform support#289
dspatig-gdx wants to merge 1 commit into
netdevops:masterfrom
dspatig-gdx:aruba-aoscx-support

Conversation

@dspatig-gdx

Copy link
Copy Markdown

Adds support for the Aruba AOS-CX platform (Platform.ARUBA_AOSCX): a new driver and config view, unit tests, and circular remediation/rollback fixtures.

AOS-CX uses a Cisco/EOS-like hierarchical CLI with no negation. Two constructs need normalization for remediation to behave correctly:

Collapsed VLAN headers — vlan 1,10 and vlan 10-12 are expanded into one vlan section each internally.
Additive trunk VLAN lists — vlan trunk allowed 10,20-22 is additive on the device, so it's normalized to one VLAN per line (remediation adds only missing VLANs), then re-collapsed to a compact range on output.
EVPN/VXLAN stanzas are treated as partial intended sections during remediation and rollback.

New driver hooks
Two hooks on HConfigDriverBase, both defaulting to a no-op (so existing drivers are unaffected):

workflow_config_post_process(config, source_config, target_config, workflow) — after remediation/rollback generation.
future_config_post_process(config) — at the end of HConfig.future().
Testing
python scripts/build.py lint-and-test passes (ruff, mypy, pyright, pylint, pytest ≥95% coverage, yamllint, flynt).

AOS-CX uses a Cisco/EOS-like hierarchical CLI with `no` negation, but two
constructs need normalization for remediation to behave: unnamed collapsed
VLAN headers (`vlan 1,10`) and additive `vlan trunk allowed` lists. Both are
expanded to one VLAN per line internally so remediation only adds the missing
VLANs, then re-collapsed on output. EVPN/VXLAN stanzas are treated as partial
intended sections so remediation and rollback don't churn unrelated lines.

To re-collapse generated configs and normalize simulated future configs
without special-casing the core workflow/future code, this adds two driver
hooks -- workflow_config_post_process() and future_config_post_process() --
that default to a no-op on HConfigDriverBase, so existing drivers are
unaffected.

Adds the driver, config view, unit tests, and circular remediation/rollback
fixtures for the new platform, and registers Platform.ARUBA_AOSCX in the
constructors and the v2->v3 os-name mapper.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread hier_config/utils.py
return TypeAdapter(tuple[TagRule, ...]).validate_python(tags_data)


def hconfig_v2_os_v3_platform_mapper(os_name: str) -> Platform:

@jtdub jtdub Jul 24, 2026

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.

This is a breaking change. Revert this.

hconfig_v2_os_v3_platform_mapper should keep its os_name: str signature and its exact-match lookup. Widening the parameter to object and coercing with str(os_name).strip().lower() changes resolution for existing callers — "IOS" previously returned Platform.GENERIC and now returns Platform.CISCO_IOS. And because object accepts anything, passing a Platform enum is now type-legal: it stringifies to "Platform.CISCO_IOS" and silently returns GENERIC, so the caller gets the wrong driver with no error. docs/utilities.md still documents the parameter as str.

If a caller needs case-insensitive or whitespace-tolerant lookup, it can normalize at its own call site. Adding a driver shouldn't be changing the v2 compatibility API.

Comment thread hier_config/workflows.py
remediation_config = self.running_config.config_to_get_to(
self.generated_config
).set_order_weight()
remediation_config = self.running_config.driver.workflow_config_post_process(

@jtdub jtdub Jul 24, 2026

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.

This is duplicating functionality that already exists.

The driver framework already provides the extension points for this — the rule models in models.py and post_load_callbacks. Threading a new driver hook through WorkflowRemediation adds a second, parallel way to do platform-specific transforms, and it's the wrong layer: it only applies to the workflow API, so running_config.config_to_get_to(generated_config) and WorkflowRemediation.remediation_config now return different remediations for identical inputs. docs/architecture.md documents those as the same computation.

More generally, a driver PR shouldn't be changing core components of hier_config. If the rule framework can't express what AOS-CX needs, raise that as an issue on its own so the extension point gets designed deliberately.

Both behaviours this hook is being used for can be handled inside the existing framework — see my comments on _prune_partial_section_workflow and on the trunk collapse in workflow_config_post_process.

Comment thread hier_config/utils.py

HCONFIG_PLATFORM_V2_TO_V3_MAPPING = {
"aruba_aoscx": Platform.ARUBA_AOSCX,
"aoscx": Platform.ARUBA_AOSCX,

@jtdub jtdub Jul 24, 2026

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.

Remove this — or at most add a single entry.

Two aliases for one platform doesn't follow the existing design pattern: every other platform has exactly one v2 OS name, and this table is treated as bidirectional. hconfig_v3_platform_v2_os_mapper iterates this same dict and returns the first key matching the platform, so as written it now returns "aruba_aoscx" for Platform.ARUBA_AOSCX — a v2 OS name that hier_config v2 never had. Downstream v2 interop gets a name no v2 install recognizes instead of the correct "generic" fallback, and tests/test_utils.py currently locks that expectation in.

If we do want AOS-CX reachable from the v2 mapper, add one key following the existing short-name convention (ios, nxos, eos, junos), and keep the reverse mapping in mind since it reads the same table.

Comment thread hier_config/root.py
if prune_empty_branches:
self._prune_emptied_branches(future_config)
return future_config
return self.driver.future_config_post_process(future_config)

@jtdub jtdub Jul 24, 2026

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.

This is well beyond the scope of adding a driver.

HConfig.future() is core behaviour and shouldn't change to accommodate a platform. Drivers need to work within the confines of the driver framework; if core functionality needs to change or the framework is missing something, raise it as an issue and we'll design it properly rather than folding it into a driver PR.

In this case the hook isn't needed at all — it exists only to undo the trunk-line collapse that workflow_config_post_process introduces. Drop the collapse and this goes back to return future_config.

As it stands it also changes what future() guarantees: it no longer returns the tree _future() built, and for AOS-CX the post-process it runs silently discards the children of any collapsed vlan X-Y section.

"""Apply driver-specific cleanup to generated workflow configs."""
return config

def future_config_post_process( # ruff:ignore[no-self-use]

@jtdub jtdub Jul 24, 2026

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.

Remove this.

Same reasoning as the hook above: HConfig.future() is core behaviour and shouldn't grow a driver hook to support a driver. Work within the driver framework; if core functionality needs to change, that's an issue to raise on its own.

This one is purely a counter-hack. It exists only because workflow_config_post_process collapsed the trunk lines in the first place, so future() has to re-split them to keep round-tripping. That's verifiable — with the collapse in place, running.future(remediation) only matches the intended config because this hook undoes it. Remove the collapse and this hook has nothing left to do.

It isn't free, either. For AOS-CX it runs _split_vlan_id_lists, so future() now silently drops the children of any collapsed vlan X-Y section, and it no longer returns the tree that _future() actually built.

return with_rule.use
return None

def workflow_config_post_process( # ruff:ignore[no-self-use]

@jtdub jtdub Jul 24, 2026

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.

Remove this.

We don't want design-pattern changes to hier_config's core in order to make a driver work. Drivers need to work within the confines of the driver framework — the rule models in models.py plus post_load_callbacks. If the framework is genuinely missing a capability, or existing functionality needs to change, raise it as an issue so the extension point gets designed deliberately. It shouldn't arrive as a side effect of a driver PR.

Concretely, this hook isn't needed. The only two things it does are:

  • Re-collapse trunk VLAN lines, which is cosmetic — see my comment on workflow_config_post_process in the AOS-CX driver.
  • Prune EVPN/VXLAN negations, which the existing sectional_overwrite_no_negate rule already expresses — see my comment on _prune_partial_section_workflow.

One further problem if it were to stay: the base declares _source_config / _target_config / _workflow, while the AOS-CX override renames them to source_config / target_config / workflow. No keyword call works against both — driver.workflow_config_post_process(config, _source_config=...) raises TypeError on the subclass, and the reverse raises on the base. The underscore prefixes also stop mypy and pyright from catching it.


return config

def future_config_post_process( # ruff:ignore[no-self-use]

@jtdub jtdub Jul 24, 2026

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.

Remove this.

Beyond the hook itself (see my comment on HConfigDriverBase.future_config_post_process), two things here:

It hand-lists the same two functions already registered in rules.post_load_callbacks a few lines up, instead of iterating them — so the two lists have to be kept in sync manually.

And it only needs to exist because the trunk collapse in workflow_config_post_process has to be undone before future() can round-trip. Drop the collapse and this method has no purpose.

],
)

def workflow_config_post_process( # ruff:ignore[no-self-use]

@jtdub jtdub Jul 24, 2026

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.

Use the existing functionality for this.

HP ProCurve already solves this exact shape of problem — compact port ranges and comma-separated VLAN lists — entirely through post_load_callbacks, and it never re-collapses on output. Remediation just emits the expanded per-port lines. AOS-CX should do the same: keep the two split callbacks (right mechanism, already registered) and drop the re-collapse pass.

Per-VLAN output is acceptable here. vlan trunk allowed is additive on the device — that's the premise of this PR — so vlan trunk allowed 40 on its own is a valid remediation. Collapsing it back into a compact range is cosmetic, and it costs a lot:

  • It doesn't emit the minimal delta. For running 10,20 vs intended 10,20,40 the remediation is vlan trunk allowed 10,20,40 — the whole target list — because children_to_collapse prefers target_allowed_children. running.config_to_get_to(intended) on the same inputs gives vlan trunk allowed 40.
  • Those two APIs now disagree. docs/architecture.md documents WorkflowRemediation as internally calling config_to_get_to, and docs/custom-workflows.md points users at the direct call, so following the docs gets you different output for the same inputs.
  • It breaks tag filtering. The collapse runs inside the remediation_config property, before callers can apply remediation tag rules, so a TagRule matching equals="vlan trunk allowed 20" matches nothing and remediation_config_filtered_text(include_tags={"trunk"}) comes back empty. A tag-scoped push silently drops the trunk change.

Drop the collapse and all three go away — along with the need for both new driver hooks.

child.delete()


def _prune_partial_section_workflow(

@jtdub jtdub Jul 24, 2026

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.

Escalating this one — it isn't just a question of where the code lives.

As written, Platform.ARUBA_AOSCX is structurally unable to remediate drift in evpn and interface vxlan *. Every no ... child is deleted from the remediation unless the intended config spells the negation out literally, so if the intended config drops a stale EVPN VLAN, the remediation comes back empty — permanently. Running config evpn / vlan 10 / vlan 20 against intended evpn / vlan 10 produces nothing at all on this branch. The same input on ARISTA_EOS correctly produces evpn / no vlan 20.

That's a site convention — "our intended EVPN stanzas are partial" — compiled into a shared driver, applying to every user of the platform with no opt-out. It's also undocumented: neither docs/drivers.md nor the CHANGELOG entry mentions it, so the surprise is silent.

On top of that, it's hardcoded imperative logic where we already have a declarative rule that covers it. sectional_overwrite_no_negate gives you the same "never negate inside this section, just re-assert the intended one" behaviour:

SectionalOverwriteNoNegateRule(match_rules=(MatchRule(equals="evpn"),))

With running evpn / arp-suppression / vlan 10 / vlan 20 and intended evpn / vlan 10 / vlan 40, that rule produces:

evpn
  vlan 10
    rd auto
  vlan 40
    rd auto

No no arp-suppression, no no vlan 20 — which is what _prune_partial_section_workflow is reaching for, in one rule, with no new hooks, no workflow string parameter, and no changes to core. Add the equivalent rule for interface vxlan and the whole pruning path can go.

If sectional_overwrite_no_negate isn't quite the semantics you need, that's a fair conversation — but it belongs in an issue proposing a new rule type that follows the existing pattern (frozen model in models.py, field on HConfigDriverRules, consumed in child.py / root.py), rather than driver-specific logic plus base-class hooks.

Minor, while it's here: workflow is a bare string compared with == "remediation", and every other value silently falls through to the rollback branch. "Remediation" or a typo gets you the wrong pruning with no error.

Comment thread hier_config/models.py
class Platform(str, Enum):
"""Enumeration of supported network operating system platforms."""

ARUBA_AOSCX = auto()

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.

Platform members are in alphabetical order — this needs to go after ARISTA_EOS, not before it (ARI sorts before ARU). Same ordering applies to the platform_drivers dict and the isinstance chain in constructors.py, and to the driver list in docs/drivers.md; all three put AOS-CX first as well.

Worth calling out the side effect, because it isn't obvious from the diff: Platform is a (str, Enum) built with auto(), so the member values are stringified ordinals, not the member names. Inserting first renumbers every existing platform:

master:   ARISTA_EOS='1'  CISCO_IOS='2'   ->  Platform('2') is CISCO_IOS
this PR:  ARUBA_AOSCX='1' ARISTA_EOS='2'  CISCO_IOS='3'  ->  Platform('2') is ARISTA_EOS

Anything that persisted a serialized Platform value deserializes to the wrong platform after upgrading, which means the wrong driver's rules generate the remediation.

Moving it to the alphabetical slot fixes the "first" part but still shifts CISCO_IOS and everything below it. If we want these values stable across releases we should assign them explicitly rather than using auto() — that's a separate change and should be raised as its own issue.

Comment thread docs/drivers.md

Aruba AOS-CX uses a Cisco IOS/EOS-like hierarchical CLI with `no ` as the negation prefix. The `ARUBA_AOSCX` driver keeps the IOS/EOS-style tree model while accounting for AOS-CX interface trunk behavior:

- `vlan trunk allowed` is additive on AOS-CX. The driver splits comma/range VLAN lists into one command per VLAN and treats the command family as additive, so remediation adds missing VLANs without removing currently allowed VLANs.

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.

This doesn't match what the driver actually does, and the same claim appears in the PR description and the CHANGELOG entry.

"remediation adds missing VLANs without removing currently allowed VLANs" — the driver's own test asserts the opposite. test_interface_vlan_trunk_allowed_is_additive expects no vlan trunk allowed 20 and no vlan trunk allowed 21 in the remediation for VLANs absent from the intended config. Removals are emitted.

"remediation only adds the missing VLANs" (CHANGELOG) isn't accurate either. Running 10,20 against intended 10,20,40 emits vlan trunk allowed 10,20,40 — the full target list, not just the missing 40 — because the collapse prefers the target's VLAN list over the delta. Harmless to apply on an additive command, but it isn't a minimal delta, and it makes the change under review look larger than it is.

Also missing here entirely: the EVPN/VXLAN partial-section behaviour, which is the most surprising thing this driver does. Whatever that ends up being after the other comments are addressed, it needs documenting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants