Skip to content

fix(ApplyDeviate): replace Exts on deviate replace, not append#4

Merged
steiler merged 1 commit into
mainfrom
fix-apply-deviate-exts-replace
Jun 15, 2026
Merged

fix(ApplyDeviate): replace Exts on deviate replace, not append#4
steiler merged 1 commit into
mainfrom
fix-apply-deviate-exts-replace

Conversation

@steiler

@steiler steiler commented Jun 8, 2026

Copy link
Copy Markdown

Overview

ApplyDeviate now copies extension statements (Exts) from deviate add and deviate replace substatements onto the deviated target Entry. Previously that propagation was missing entirely, so extensions declared inside a deviation (for example sdcio-ext:sensitive in a device-profile overlay) never appeared on the parsed leaf entry.

Motivation

RFC 7950 allows extension substatements under deviate add and deviate replace. The parser already attached them to the deviation-side Entry (devSpec.Exts), but ApplyDeviate never merged them into deviatedNode.Exts, so downstream consumers could not see them.

SDCIO relies on this for marking sensitive leaves via deviations; see related work in sdc-protos#123, schema-server#244, and data-server#452.

What changed

Inside the existing case DeviationAdd, DeviationReplace handling (alongside Default, must, etc.), add explicit Exts handling mirroring the Default pattern:

  • DeviationAdd: deviatedNode.Exts = append(deviatedNode.Exts, devSpec.Exts...)
  • DeviationReplace: deviatedNode.Exts = append([]*Statement{}, devSpec.Exts...) — the target node's extension list becomes exactly the deviation's list (replace semantics), not a merge that keeps unrelated prior extensions from accumulating incorrectly when the spec intends a full substitution of the deviation-supplied set.
switch dt {
case DeviationAdd:
	deviatedNode.Exts = append(deviatedNode.Exts, devSpec.Exts...)
case DeviationReplace:
	deviatedNode.Exts = append([]*Statement{}, devSpec.Exts...)
}

Tests

  • deviate add propagates extensions onto target entry — extensions from deviate add reach the target leaf.
  • deviate replace propagates extensions onto target entry — extensions from deviate replace reach the target leaf.
  • deviate replace replaces existing extensions, does not accumulate — base leaf already has tag-ext:old-tag; overlay deviate replace supplies sdcio-ext:sensitive; result must not retain the old extension.

Test plan

  • go test ./pkg/yang/... (or full module test as per repo CI)
  • Confirm downstream PRs that pin this commit still build and pass sensitive-path / schema tests

DeviationReplace was incorrectly appending extensions to the existing
slice instead of replacing them.  The Default field already handles this
distinction correctly; apply the same pattern to Exts:

  DeviationAdd     -> append(deviatedNode.Exts, devSpec.Exts...)
  DeviationReplace -> append([]*Statement{}, devSpec.Exts...)

Add a test case that verifies a pre-existing extension is discarded
when deviate replace supplies a different one.

Co-authored-by: Cursor <cursoragent@cursor.com>

@alexandernorth alexandernorth left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm

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