SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

updatectl: fix feature JSON parsing

Browse Source 
The feature JSON payload switched to `id`, `isEnabled`, and `appstream`,
and made `documentation` an array of URLs.
`updatectl` still parsed legacy table-derived names and a single URL.
This broke feature JSON parsing and made output inconsistent.
Features without documentation could also cause a null dereference.

Align `updatectl` with the current schema and preserve all
documentation URLs across sysupdate, D-Bus, and Varlink payloads.
Propagate condition evaluation errors through `condition_test_list()`
and `feature_is_suggested()`, with consistent JSON error handling and
tests.
The `suggested` field is now optional and always a boolean when
present; it is omitted rather than set to an error string when
SuggestOn*= conditions cannot be evaluated.
Add `suggested` metadata and document the field transition.

Follow-up for 3e34ccf6df.
Follow-up for 75a247d4de.
Follow-up for 40e727c93b.

Fixes #43603
This commit is contained in:
dongshengyuan
2026-09-08 16:05:36 +02:00
committed by Zbigniew Jędrzejewski-Szmek
parent 8839a9810e
commit a6f7ded859
10 changed files with 206 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/sysupdate/sysupdate-feature.c
+19 -18
View File
@@ -8,6 +8,7 @@
#include "hash-funcs.h"
#include "path-util.h"
#include "string-util.h"
#include "strv.h"
#include "sysupdate-config.h"
#include "sysupdate-feature.h"
#include "sysupdate-util.h"
@@ -19,7 +20,7 @@ static Feature *feature_free(Feature *f) {
free(f->id);
free(f->description);
free(f->documentation);
strv_free(f->documentation);
free(f->appstream);
condition_free_list(f->suggest_on);
@@ -72,22 +73,22 @@ int feature_read_definition(Feature *f, const char *root, const char *path, cons
return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Malformed feature filename '%s': %m", filename);
ConfigTableItem table[] = {
{ "Feature", "Description", config_parse_string, 0, &f->description },
{ "Feature", "Documentation", config_parse_url_specifiers, 0, &f->documentation },
{ "Feature", "AppStream", config_parse_url_specifiers, 0, &f->appstream },
{ "Feature", "Enabled", config_parse_bool, 0, &f->enabled },
{ "Feature", "Suggest", config_parse_tristate, 0, &f->suggest },
{ "Feature", "SuggestOnArchitecture", config_parse_condition, CONDITION_ARCHITECTURE, &f->suggest_on },
{ "Feature", "SuggestOnFirmware", config_parse_condition, CONDITION_FIRMWARE, &f->suggest_on },
{ "Feature", "SuggestOnVirtualization", config_parse_condition, CONDITION_VIRTUALIZATION, &f->suggest_on },
{ "Feature", "SuggestOnHost", config_parse_condition, CONDITION_HOST, &f->suggest_on },
{ "Feature", "SuggestOnFraction", config_parse_condition, CONDITION_FRACTION, &f->suggest_on },
{ "Feature", "SuggestOnKernelCommandLine", config_parse_condition, CONDITION_KERNEL_COMMAND_LINE, &f->suggest_on },
{ "Feature", "SuggestOnVersion", config_parse_condition, CONDITION_VERSION, &f->suggest_on },
{ "Feature", "SuggestOnCredential", config_parse_condition, CONDITION_CREDENTIAL, &f->suggest_on },
{ "Feature", "SuggestOnSecurity", config_parse_condition, CONDITION_SECURITY, &f->suggest_on },
{ "Feature", "SuggestOnOSRelease", config_parse_condition, CONDITION_OS_RELEASE, &f->suggest_on },
{ "Feature", "SuggestOnMachineTag", config_parse_condition, CONDITION_MACHINE_TAG, &f->suggest_on },
{ "Feature", "Description", config_parse_string, 0, &f->description },
{ "Feature", "Documentation", config_parse_url_specifiers_many, 0, &f->documentation },
{ "Feature", "AppStream", config_parse_url_specifiers, 0, &f->appstream },
{ "Feature", "Enabled", config_parse_bool, 0, &f->enabled },
{ "Feature", "Suggest", config_parse_tristate, 0, &f->suggest },
{ "Feature", "SuggestOnArchitecture", config_parse_condition, CONDITION_ARCHITECTURE, &f->suggest_on },
{ "Feature", "SuggestOnFirmware", config_parse_condition, CONDITION_FIRMWARE, &f->suggest_on },
{ "Feature", "SuggestOnVirtualization", config_parse_condition, CONDITION_VIRTUALIZATION, &f->suggest_on },
{ "Feature", "SuggestOnHost", config_parse_condition, CONDITION_HOST, &f->suggest_on },
{ "Feature", "SuggestOnFraction", config_parse_condition, CONDITION_FRACTION, &f->suggest_on },
{ "Feature", "SuggestOnKernelCommandLine", config_parse_condition, CONDITION_KERNEL_COMMAND_LINE, &f->suggest_on },
{ "Feature", "SuggestOnVersion", config_parse_condition, CONDITION_VERSION, &f->suggest_on },
{ "Feature", "SuggestOnCredential", config_parse_condition, CONDITION_CREDENTIAL, &f->suggest_on },
{ "Feature", "SuggestOnSecurity", config_parse_condition, CONDITION_SECURITY, &f->suggest_on },
{ "Feature", "SuggestOnOSRelease", config_parse_condition, CONDITION_OS_RELEASE, &f->suggest_on },
{ "Feature", "SuggestOnMachineTag", config_parse_condition, CONDITION_MACHINE_TAG, &f->suggest_on },
{}
};
@@ -111,7 +112,7 @@ int feature_read_definition(Feature *f, const char *root, const char *path, cons
return 0;
}
int feature_is_suggested(Feature *f) {
int feature_is_suggested(const Feature *f) {
assert(f);
if (f->suggest >= 0)
src/sysupdate/sysupdate-feature.h
+2 -2
View File
@@ -9,7 +9,7 @@ typedef struct Feature {
char *id;
char *description;
char *documentation;
char **documentation;
char *appstream;
bool enabled;
@@ -27,4 +27,4 @@ extern const struct hash_ops feature_hash_ops;
int feature_read_definition(Feature *f, const char *root, const char *path, const char *const *conf_file_dirs);
int feature_is_suggested(Feature *f);
int feature_is_suggested(const Feature *f);
src/sysupdate/sysupdate.c
+31 -12
View File
@@ -2019,6 +2019,8 @@ static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) {
}
}
static int feature_to_json(Context *context, const Feature *f, sd_json_variant **ret);
VERB(verb_features, "features", "[FEATURE]\0", VERB_ANY, 2, 0,
"Show optional features");
static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata) {
@@ -2056,6 +2058,20 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata
"Optional feature not found: %s",
feature_id);
if (sd_json_format_enabled(arg_json_format_flags)) {
_cleanup_(sd_json_variant_unrefp) sd_json_variant *json = NULL;
r = feature_to_json(&context, f, &json);
if (r < 0)
return r;
r = sd_json_variant_dump(json, arg_json_format_flags, stdout, NULL);
if (r < 0)
return log_error_errno(r, "Failed to print JSON: %m");
return 0;
}
table = table_new_vertical();
if (!table)
return log_oom();
@@ -2092,11 +2108,10 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata
return table_log_add_error(r);
}
if (f->documentation) {
if (!strv_isempty(f->documentation)) {
r = table_add_many(table,
TABLE_FIELD, "Documentation",
TABLE_STRING, f->documentation,
TABLE_SET_URL, f->documentation);
TABLE_STRV_WRAPPED, f->documentation);
if (r < 0)
return table_log_add_error(r);
}
@@ -2142,8 +2157,7 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata
r = table_add_many(table,
TABLE_STRING, f->id,
TABLE_STRING, f->description,
TABLE_STRING, f->documentation,
TABLE_SET_URL, f->documentation);
TABLE_STRV_WRAPPED, f->documentation);
if (r < 0)
return table_log_add_error(r);
}
@@ -2248,8 +2262,10 @@ static int context_enable_feature(
Feature *f;
HASHMAP_FOREACH(f, c->features) {
r = feature_is_suggested(f);
if (r < 0)
return log_error_errno(r, "Failed to determine if feature '%s' of component '%s' is suggested: %m", f->id, context_component_display(c));
if (r < 0) {
log_warning_errno(r, "Failed to determine if feature '%s' of component '%s' is suggested, skipping: %m", f->id, context_component_display(c));
continue;
}
if (r == 0) {
log_debug("Feature '%s' of component '%s' is not suggested, skipping.", f->id, context_component_display(c));
continue;
@@ -2404,7 +2420,7 @@ static int verb_enable_feature(int argc, char *argv[], uintptr_t _data, void *us
static int feature_to_json(Context *context, const Feature *f, sd_json_variant **ret) {
_cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL;
_cleanup_strv_free_ char **transfers = NULL;
const char *documentation_strv[2] = { NULL, };
int suggested;
int r;
assert(context);
@@ -2415,17 +2431,20 @@ static int feature_to_json(Context *context, const Feature *f, sd_json_variant *
if (r < 0)
return r;
/* FIXME: Long term we’d like to support an array of documentation, but currently the D-Bus interface
* doesn’t support that and neither do the internals of sysupdate. So just expose 0 or 1 URLs for now. */
documentation_strv[0] = f->documentation;
suggested = feature_is_suggested(f);
if (suggested < 0)
log_debug_errno(suggested,
"Failed to determine whether feature '%s' is suggested, omitting field: %m",
f->id);
r = sd_json_variant_merge_objectbo(
&v,
SD_JSON_BUILD_PAIR_STRING("id", f->id),
JSON_BUILD_PAIR_STRING_NON_EMPTY("description", f->description),
JSON_BUILD_PAIR_STRV_NON_EMPTY("documentation", (char **) documentation_strv),
JSON_BUILD_PAIR_STRV_NON_EMPTY("documentation", f->documentation),
JSON_BUILD_PAIR_STRING_NON_EMPTY("appstream", f->appstream),
SD_JSON_BUILD_PAIR_BOOLEAN("isEnabled", f->enabled),
SD_JSON_BUILD_PAIR_CONDITION(suggested >= 0, "suggested", SD_JSON_BUILD_BOOLEAN(suggested > 0)),
JSON_BUILD_PAIR_STRV_NON_EMPTY("transfers", transfers));
if (r < 0)
return log_oom();
src/sysupdate/updatectl.c
+47 -21
View File
@@ -1386,18 +1386,26 @@ static int verb_vacuum(int argc, char *argv[], uintptr_t _data, void *userdata)
}
typedef struct Feature {
char *name;
char *id;
char *description;
bool enabled;
char *documentation;
int suggested;
char **documentation;
char *appstream;
char **transfers;
} Feature;
#define FEATURE_NULL \
(Feature) { \
.suggested = -1, \
}
static void feature_done(Feature *f) {
assert(f);
f->name = mfree(f->name);
f->id = mfree(f->id);
f->description = mfree(f->description);
f->documentation = mfree(f->documentation);
f->documentation = strv_free(f->documentation);
f->appstream = mfree(f->appstream);
f->transfers = strv_free(f->transfers);
}
@@ -1405,16 +1413,18 @@ static int describe_feature(sd_bus *bus, const char *feature, Feature *ret) {
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
_cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
_cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL;
_cleanup_(feature_done) Feature f = {};
_cleanup_(feature_done) Feature f = FEATURE_NULL;
char *json;
int r;
static const sd_json_dispatch_field dispatch_table[] = {
{ "name", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(Feature, name), SD_JSON_MANDATORY },
{ "description", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(Feature, description), 0 },
{ "enabled", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(Feature, enabled), SD_JSON_MANDATORY },
{ "documentationUrl", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(Feature, documentation), 0 },
{ "transfers", SD_JSON_VARIANT_ARRAY, sd_json_dispatch_strv, offsetof(Feature, transfers), 0 },
{ "id", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(Feature, id), SD_JSON_MANDATORY },
{ "description", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(Feature, description), SD_JSON_NULLABLE },
{ "isEnabled", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_stdbool, offsetof(Feature, enabled), SD_JSON_MANDATORY },
{ "suggested", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_tristate, offsetof(Feature, suggested), SD_JSON_NULLABLE },
{ "documentation", SD_JSON_VARIANT_ARRAY, sd_json_dispatch_strv, offsetof(Feature, documentation), SD_JSON_NULLABLE },
{ "appstream", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(Feature, appstream), SD_JSON_NULLABLE },
{ "transfers", SD_JSON_VARIANT_ARRAY, sd_json_dispatch_strv, offsetof(Feature, transfers), SD_JSON_NULLABLE },
{}
};
@@ -1443,7 +1453,7 @@ static int describe_feature(sd_bus *bus, const char *feature, Feature *ret) {
if (r < 0)
return log_error_errno(r, "Failed to parse JSON: %m");
r = sd_json_dispatch(v, dispatch_table, 0, &f);
r = sd_json_dispatch(v, dispatch_table, SD_JSON_ALLOW_EXTENSIONS, &f);
if (r < 0)
return log_error_errno(r, "Failed to dispatch JSON: %m");
@@ -1481,15 +1491,15 @@ static int list_features(sd_bus *bus) {
return bus_log_parse_error(r);
STRV_FOREACH(feature, features) {
_cleanup_(feature_done) Feature f = {};
_cleanup_(feature_done) Feature f = FEATURE_NULL;
_cleanup_free_ char *name_link = NULL;
r = describe_feature(bus, *feature, &f);
if (r < 0)
return r;
if (urlify_enabled() && f.documentation) {
name_link = strjoin(f.name, glyph(GLYPH_EXTERNAL_LINK));
if (urlify_enabled() && !strv_isempty(f.documentation)) {
name_link = strjoin(f.id, glyph(GLYPH_EXTERNAL_LINK));
if (!name_link)
return log_oom();
}
@@ -1497,8 +1507,8 @@ static int list_features(sd_bus *bus) {
r = table_add_many(table,
TABLE_BOOLEAN_CHECKMARK, f.enabled,
TABLE_SET_COLOR, ansi_highlight_green_red(f.enabled),
TABLE_STRING, name_link ?: f.name,
TABLE_SET_URL, f.documentation,
TABLE_STRING, name_link ?: f.id,
TABLE_SET_URL, strv_isempty(f.documentation) ? NULL : f.documentation[0],
TABLE_STRING, f.description);
if (r < 0)
return table_log_add_error(r);
@@ -1512,7 +1522,7 @@ VERB(verb_features, "features", "[FEATURE]\0", VERB_ANY, 2, VERB_ONLINE_ONLY,
static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata) {
sd_bus *bus = ASSERT_PTR(userdata);
_cleanup_(table_unrefp) Table *table = NULL;
_cleanup_(feature_done) Feature f = {};
_cleanup_(feature_done) Feature f = FEATURE_NULL;
int r;
if (argc == 1)
@@ -1528,23 +1538,39 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata
r = table_add_many(table,
TABLE_FIELD, "Name",
TABLE_STRING, f.name,
TABLE_STRING, f.id,
TABLE_FIELD, "Enabled",
TABLE_BOOLEAN, f.enabled);
if (r < 0)
return table_log_add_error(r);
if (f.suggested >= 0) {
r = table_add_many(table,
TABLE_FIELD, "Suggested",
TABLE_BOOLEAN, f.suggested);
if (r < 0)
return table_log_add_error(r);
}
if (f.description) {
r = table_add_many(table, TABLE_FIELD, "Description", TABLE_STRING, f.description);
if (r < 0)
return table_log_add_error(r);
}
if (f.documentation) {
if (!strv_isempty(f.documentation)) {
r = table_add_many(table,
TABLE_FIELD, "Documentation",
TABLE_STRING, f.documentation,
TABLE_SET_URL, f.documentation);
TABLE_STRV_WRAPPED, f.documentation);
if (r < 0)
return table_log_add_error(r);
}
if (f.appstream) {
r = table_add_many(table,
TABLE_FIELD, "AppStream",
TABLE_STRING, f.appstream,
TABLE_SET_URL, f.appstream);
if (r < 0)
return table_log_add_error(r);
}