Skip to content

Fix stale error leak in EnsureBGP causing "gnmiext: nil"#386

Merged
felix-kaestner merged 1 commit into
mainfrom
fix/bgp-nil
May 29, 2026
Merged

Fix stale error leak in EnsureBGP causing "gnmiext: nil"#386
felix-kaestner merged 1 commit into
mainfrom
fix/bgp-nil

Conversation

@felix-kaestner
Copy link
Copy Markdown
Contributor

When the BGP domain does not exist on the device (first-time config) and the ASN uses plain notation, the outer err variable retained the ErrNil value from the domain GetConfig call. Neither switch case for AS format fired, so err was never reassigned and leaked to the caller.

Scope error handling into each switch case so that the shared err variable is no longer carried across unrelated operations.

Also skip the router-ID conflict check when the existing domain has an empty router-ID, avoiding a false-positive mismatch error.

@felix-kaestner felix-kaestner requested a review from a team as a code owner May 29, 2026 12:44
When the BGP domain does not exist on the device (first-time config)
and the ASN uses plain notation, the outer err variable retained the
ErrNil value from the domain GetConfig call. Neither switch case for
AS format fired, so err was never reassigned and leaked to the caller.

Scope error handling into each switch case so that the shared err
variable is no longer carried across unrelated operations.

Also skip the router-ID conflict check when the existing domain has
an empty router-ID, avoiding a false-positive mismatch error.

Signed-off-by: Felix Kästner <felix.kaestner@sap.com>
@felix-kaestner felix-kaestner enabled auto-merge (squash) May 29, 2026 12:45
Copy link
Copy Markdown

@SchwarzM SchwarzM left a comment

Choose a reason for hiding this comment

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

lgtm

@felix-kaestner felix-kaestner disabled auto-merge May 29, 2026 12:47
@github-actions
Copy link
Copy Markdown

Merging this branch will not change overall coverage

Impacted Packages Coverage Δ 🤖
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos 9.95% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/provider.go 0.06% (ø) 1779 1 1778

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

@felix-kaestner felix-kaestner merged commit 58aced2 into main May 29, 2026
20 checks passed
@felix-kaestner felix-kaestner deleted the fix/bgp-nil branch May 29, 2026 12:54
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