Skip to content

[code sync] Merge code from sonic-net/sonic-utilities:202605 to 202607 - #443

Merged
mssonicbld merged 3 commits into
Azure:202607from
mssonicbld:sonicbld/202607-merge
Aug 27, 2026
Merged

[code sync] Merge code from sonic-net/sonic-utilities:202605 to 202607#443
mssonicbld merged 3 commits into
Azure:202607from
mssonicbld:sonicbld/202607-merge

Conversation

@mssonicbld

Copy link
Copy Markdown
Collaborator
* 2c44f94f - (origin/202605) Fix migrate_sonic_packages() resolv.conf injection for relative symlinks (#4790) (2026-08-26) [mssonicbld]
* c15e7261 - [click]: Add resilient parsing for AbbreviationGroup (#4782) (2026-08-26) [mssonicbld]<br>```

mssonicbld and others added 3 commits August 26, 2026 13:14
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>

<!--
 Please make sure you've read and understood our contributing guidelines:
 https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

 CODE_OF_CONDUCT.md LICENSE README.md SECURITY.md SUPPORT.md azure-pipelines failure_prs.log scripts skip_prs.log Make sure all your commits include a signature generated with `git commit -s` **

 If this is a bug fix, make sure your description includes "closes #xxxx",
 "fixes #xxxx" or "resolves #xxxx" so that GitHub automatically closes the related
 issue when the PR is merged.

 If you are adding/modifying/removing any command or utility script, please also
 make sure to add/modify/remove any unit tests from the tests
 directory as appropriate.

 If you are modifying or removing an existing 'show', 'config' or 'sonic-clear'
 subcommand, or you are adding a new subcommand, please make sure you also
 update the Command Line Reference Guide (doc/Command-Reference.md) to reflect
 your changes.

 Please provide the following information:
-->

## The bug

The `config` command uses a custom Click group, `AbbreviationGroup`, that supports abbreviated subcommand names via prefix matching:

`sonic-buildimage/src/sonic-utilities/utilities_common/cli.py`
```python
 def get_command(self, ctx, cmd_name):
 # Try to get builtin commands as normal
 rv = click.Group.get_command(self, ctx, cmd_name)
 if rv is not None:
 return rv
 ...
 matches = []
 shortest = None
 for x in self.list_commands(ctx):
 if x.lower().startswith(cmd_name.lower()):
 matches.append(x)
 ...
 if not matches:
 return None
 elif len(matches) == 1:
 return click.Group.get_command(self, ctx, matches[0])
 else:
 for x in matches:
 if not x.startswith(shortest):
 break
 else:
 return click.Group.get_command(self, ctx, shortest)

 ctx.fail('Too many matches: %s' % ', '.join(sorted(matches)))
```

`switch` is an ambiguous prefix — it matches `switch-fast-linkup`, `switch-hash`, `switch-trimming`, and `switchport`. The shortest match is `switchport`, but not all matches start with `switchport`, so the `for/else` falls through to `ctx.fail(...)`, which raises `click.exceptions.UsageError`.

## Why the trailing space matters

The two Tab presses take completely different code paths inside Click:

- **`config switch` + Tab** (cursor right after the word): Click treats `switch` as the **incomplete** token. It just filters the list of subcommand names by that prefix and prints the candidates. `get_command("switch")` is never called, so nothing fails — that's why we see the nice list of 4 completions.

- **`config switch ` + Tab** (extra space): now `switch` is a **complete** argument and the incomplete token is `""`. To figure out which subcommand's arguments to complete next, Click's `_resolve_context` walks the arg list and calls `command.resolve_command(ctx, ["switch"])` → `get_command(ctx, "switch")`. That triggers the ambiguous-prefix path and hits `ctx.fail()`.

The real defect is that `ctx.fail()` raises an exception **during shell completion**. Click's completion resolver doesn't catch exceptions from `get_command`, so instead of silently returning "no completions," the whole traceback gets dumped into the terminal.

## Why it only appears now

During completion, Click sets `ctx.resilient_parsing = True` precisely so that resolution logic degrades gracefully instead of erroring. `AbbreviationGroup.get_command` ignores that flag. (We are also on a newer Click / Python 3.13, where completion resolution is stricter — older versions were more forgiving, which is likely why we hadn't seen this before.)

## The fix

Guard the failure so it returns `None` during completion instead of raising:

```python
 else:
 for x in matches:
 if not x.startswith(shortest):
 break
 else:
 return click.Group.get_command(self, ctx, shortest)

 if ctx.resilient_parsing:
 return None

 ctx.fail('Too many matches: %s' % ', '.join(sorted(matches)))
```

With that, tab-completion on an ambiguous prefix just yields no completion (or the caller can list children) instead of crashing, while actually *running* `config switch` still gives the proper "Too many matches" usage error.

#### What I did
* Fixed traceback dumping into the terminal on command autocompletion

#### How I did it
* Added a guard for `AbbreviationGroup` autocompletion

#### How to verify it
1. Run UTs

#### Previous command output (if the output of a command-line utility has changed)
```
root@sonic:/home/admin# config switch Traceback (most recent call last):
 File "/usr/local/bin/config", line 8, in <module>
 sys.exit(config())
 ~~~~~~^^
 File "/usr/lib/python3/dist-packages/click/core.py", line 1161, in __call__
 return self.main(*args, **kwargs)
 ~~~~~~~~~^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3/dist-packages/click/core.py", line 1077, in main
 self._main_shell_completion(extra, prog_name, complete_var)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3/dist-packages/click/core.py", line 1156, in _main_shell_completion
 rv = shell_complete(self, ctx_args, prog_name, complete_var, instruction)
 File "/usr/lib/python3/dist-packages/click/shell_completion.py", line 49, in shell_complete
 echo(comp.complete())
 ~~~~~~~~~~~~~^^
 File "/usr/lib/python3/dist-packages/click/shell_completion.py", line 293, in complete
 completions = self.get_completions(args, incomplete)
 File "/usr/lib/python3/dist-packages/click/shell_completion.py", line 273, in get_completions
 ctx = _resolve_context(self.cli, self.ctx_args, self.prog_name, args)
 File "/usr/lib/python3/dist-packages/click/shell_completion.py", line 525, in _resolve_context
 name, cmd, args = command.resolve_command(ctx, args)
 ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
 File "/usr/lib/python3/dist-packages/click/core.py", line 1738, in resolve_command
 cmd = self.get_command(ctx, cmd_name)
 File "/usr/local/lib/python3.13/dist-packages/utilities_common/cli.py", line 59, in get_command
 ctx.fail('Too many matches: %s' % ', '.join(sorted(matches)))
 ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3/dist-packages/click/core.py", line 691, in fail
 raise UsageError(message, self)
click.exceptions.UsageError: Too many matches: switch-fast-linkup, switch-hash, switch-trimming, switchport

Usage: config [OPTIONS] COMMAND [ARGS]...
Try 'config -h' for help.

Error: Too many matches: switch-fast-linkup, switch-hash, switch-trimming, switchport
```

#### New command output (if the output of a command-line utility has changed)
```
root@sonic:/home/admin# config switch
switch-fast-linkup
switch-hash
switch-trimming
switchport
...
```

#### A picture of a cute animal (not mandatory but encouraged)
```
 .---. .-----------
 / \ __ / ------
 / / \( )/ -----
 ////// ' \/ ` ---
 //// / // : : ---
 // / / /` '--
// //..\\
 ====UU====UU====
 '//||\\`
 ''``
```

Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>
…nks (#4790)

#### What I did

Fixed `sonic-installer install` failing in `migrate_sonic_packages()` with `Temporary failure in name resolution` when `sonic-package-manager migrate` has to fetch a package manifest from a container registry inside the new image's chroot.

#4365 made the DNS injection symlink-aware, but its path math only handles absolute symlink targets: `lstrip("/")` is a no-op on a relative target such as `../run/resolvconf/resolv.conf` (the form the Debian resolvconf package ships in the image), so `os.path.join()` produced a path whose `..` escaped the chroot mount. The host's DNS configuration was written to the host filesystem (`/tmp/run/...`) instead of into the chroot overlay, leaving the chroot with whatever stale `resolv.conf` the image's squashfs happened to carry from its build server — so package migration worked or failed depending on which build machine produced the image.

#### How I did it

- Resolve a relative symlink target against the symlink's own directory (`etc/`), per POSIX symlink semantics.
- `os.path.normpath()` on the anchored absolute path collapses `..` and clamps it at the chroot root, mirroring how the kernel resolves the link inside the chroot — the computed path can never escape the image mount.
- `cp -L --remove-destination`: if the computed target already exists as a symlink, replace it instead of writing through it.
- Absolute targets pass through unchanged (`normpath` is the identity on them), so the case #4365 fixed behaves exactly as before; the regular-file branch is untouched.

#### How to verify it

- `tests/test_sonic_installer.py`: `test_install` is parametrized over the new image's `/etc/resolv.conf` layouts — absolute symlink, relative symlink, a target with excess `..`, and a regular file. The relative-symlink case fails against the previous code; the expected command list is asserted with strict order/argv equality.
- Verified end to end on a Mellanox MSN4700: installing an image whose squashfs ships the relative symlink and stale, unreachable build-time nameservers previously failed at the `sonic-package-manager migrate` step with the DNS error above (and left a stray copy of the host's DNS configuration under `/tmp/run/` on the host). With this fix the same install completes: the host DNS lands inside the chroot overlay (`rw/run/resolvconf/resolv.conf`) and no stray host file is created.

Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@mssonicbld
mssonicbld merged commit 407e5c5 into Azure:202607 Aug 27, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant