Skip to content

fix(snap): make url options consistent with landscape-config#430

Merged
wck0 merged 1 commit intocanonical:mainfrom
st3v3nmw:consistent-urls
Apr 20, 2026
Merged

fix(snap): make url options consistent with landscape-config#430
wck0 merged 1 commit intocanonical:mainfrom
st3v3nmw:consistent-urls

Conversation

@st3v3nmw
Copy link
Copy Markdown
Member

@st3v3nmw st3v3nmw commented Apr 17, 2026

Field reported that after running snap set landscape-client ping-url=... and other keys, only ping-url remained visible in snap get after restart with no configuration applied. Users naturally reach for url and ping-url, consistent with landscape-client.conf, the landscape-config CLI, and the snap's own landscape-client.config, but the hook only recognized landscape-url and landscape-ping-url, so those keys were never applied or cleared.

Additionally, the hook appended /message-system and /ping to the URL internally, meaning users following any existing documentation (which mostly uses full URLs) would end up with doubled paths in the configuration.

This PR renames the snap options to url and ping-url, and removes the internal path appending so full URLs are passed through directly. The old landscape-url and landscape-ping-url keys are kept as deprecated aliases so existing setups continue to work.

Related: canonical/landscape-documentation#192


1. After a fresh installation, the default-configure hook works as expected:

$ cat /var/snap/landscape-client/common/etc/landscape-client.conf
[client]
account_name =
computer_title =
url = https://landscape.canonical.com/message-system
ping_url = http://landscape.canonical.com/ping
log_level = info
script_users = ALL
manager_plugins = ProcessKiller,UserManager,ShutdownManager,HardwareInfo,KeystoneToken,SnapManager,SnapServicesManager,ScriptExecution,UbuntuProInfo
monitor_plugins = ActiveProcessInfo,ComputerInfo,LoadAverage,MemoryInfo,MountInfo,ProcessorInfo,Temperature,UserMonitor,RebootRequired,NetworkActivity,NetworkDevice,CPUUsage,SwiftUsage,CephUsage,ComputerTags,SnapServicesMonitor,CloudInit

2. New url and ping-url keys are applied and cleared:

$ snap set landscape-client url=https://new.example.com/message-system ping-url=http://new.example.com/ping
$ snap get -d landscape-client
{}
$ grep -E "url|ping_url" /var/snap/landscape-client/common/etc/landscape-client.conf
url = https://new.example.com/message-system
ping_url = http://new.example.com/ping

3. Legacy landscape-url & landscape-ping-url still work:

$ snap set landscape-client landscape-url=https://legacy-url.example.com
$ snap set landscape-client landscape-ping-url=http://legacy-ping.example.com
$ snap get -d landscape-client
{}
$ grep -E "url|ping_url" /var/snap/landscape-client/common/etc/landscape-client.conf
url = https://legacy-url.example.com/message-system
ping_url = http://legacy-ping.example.com/ping

4. New key takes precedence over legacy key when both are set

$ snap set landscape-client landscape-url=https://old.example.com url=https://override.example.com/message-system
$ snap get -d landscape-client
{}
$ grep "url" /var/snap/landscape-client/common/etc/landscape-client.conf
url = https://override.example.com/message-system

5. Unknown key persists (documented behavior)

$ snap set landscape-client some-url=https://typo.example.com
$ snap get -d landscape-client
{
        "some-url": "https://typo.example.com"
}
$ snap unset landscape-client some-url
$ snap get -d landscape-client
{}

snapctl get (used inside the hooks) has no equivalent to snap get -d landscape-client that returns the full config document, so the hook can only unset keys it recognizes. Unknown keys set by mistake will persist in snap options and have no effect. This behavior is now documented.

@st3v3nmw st3v3nmw force-pushed the consistent-urls branch 2 times, most recently from 7970f8f to 5941516 Compare April 17, 2026 07:57
@st3v3nmw st3v3nmw changed the title fix(snap): make config options consistent with landscape-config fix(snap): make url options consistent with landscape-config Apr 17, 2026
@st3v3nmw st3v3nmw marked this pull request as ready for review April 17, 2026 09:44
@st3v3nmw st3v3nmw requested review from Perfect5th and wck0 April 17, 2026 09:46
@wck0
Copy link
Copy Markdown
Contributor

wck0 commented Apr 17, 2026

The snap works as advertised. Reviewing the code now.

Copy link
Copy Markdown
Contributor

@wck0 wck0 left a comment

Choose a reason for hiding this comment

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

Changes look good, thanks!
One question about potentially logging a warning.

Comment thread snap/hooks/configure
@wck0 wck0 merged commit f7c075d into canonical:main Apr 20, 2026
7 checks passed
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