Skip to content

confdb: support constrained set and unset [WIP] - #17709

Draft
miguelpires wants to merge 2 commits into
canonical:masterfrom
miguelpires:spike-write-filtering
Draft

miguelpires wants to merge 2 commits into
canonical:masterfrom
miguelpires:spike-write-filtering

Conversation

@miguelpires

Copy link
Copy Markdown
Contributor

This is a proposal on how to support writing and removing confdb data while providing constraints. A filtered/constrained write should replace the entries that match the constraints with the supplied data. Writing to a request path with some constraints and then reading the same path with the same constraints should return the same data that was set. For example:

// initial data
$ snap get system/validation-sets/state -d sets
{
  "v1": {
    "sets": [
      {
        "account-id": "other-a",
        "validation-set": "keep-a"
      },
      {
        "account-id": "my-acc",
        "validation-set": "old-1"
      }
    ]
  }
}

// the constraining field is set in the value automatically
$ snap set system/validation-sets/admin --with account-id=my-acc -t 'sets=[{"validation-set":"new-1"}]' 

$ snap get system/validation-sets/state -d sets
{
  "v1": {
    "sets": [
      {
        "account-id": "other-a",
        "validation-set": "keep-a"
      },
      {
        "account-id": "my-acc",
        "validation-set": "new-1"
      }
    ]
  }
}

I experimented with different ways of implementing this because some were quite messy due to some inconsistencies with how we did things before. As a result, Set() and Unset() implement things differently. Set() does a read of the path with inverted constraints (it reads the data that doesn't match them), merges that with the user provided data and writes that. The result is that the existing data that does match the constraints is replaced. This avoids doing a traversal of the existing data while merging new data which can be tricky, especially in the case of lists where write order can reshape the end result. Unset() simply traverses the databag as before and applies constraints before actually going through with the data removal. It's a bit simpler since there's no data to merge.

https://warthogs.atlassian.net/browse/SNAPDENG-37194

This correct the ordering of path operations to be aware of whether
we're writing or removing data, as appropriate. It also changes the
ordering path removals regarding lists to be the opposite of the writes.
For instance, we want to remove list[2] before list[1].field, for the
same reason we do the write in the opposite order - to avoid changing
the meaning of subsequent operations. The one case where operations
should be sorted in a way that ignores whether it's a write or removal
is when the paths are ancestor/descendant, in that case the more
specific order should "win". This also filters unsets of paths fully
included in more general unsets since removing either list[1] or
list[1].field can both result in list[1] being fully removed which then
changes the meaning of whichever path is removed after.

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
@miguelpires miguelpires added the confdb confdb work (previously called registries and before aspects) label Sep 24, 2026
@github-actions github-actions Bot added Needs Documentation -auto- Label automatically added which indicates the change needs documentation Run only one system Only runs spread tests on one system labels Sep 24, 2026
@miguelpires miguelpires changed the title confdb: support constrained set and unset confdb: support constrained set and unset [WIP] Sep 24, 2026
@github-actions

Copy link
Copy Markdown

Thu Sep 24 16:31:20 UTC 2026
The following results are from: https://github.com/canonical/snapd/actions/runs/36025000087

No spread result artifacts found

No spread results JSON artifacts were available for this workflow run, so spread failures (if any) could not be reported.

@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.57464% with 139 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.43%. Comparing base (55d3556) to head (7a697a2).
⚠️ Report is 22 commits behind head on master.

Files with missing lines Patch % Lines
confdb/confdb.go 71.77% 77 Missing and 39 partials ⚠️
overlord/confdbstate/transaction.go 68.88% 9 Missing and 5 partials ⚠️
overlord/confdbstate/confdbstate.go 71.42% 4 Missing and 2 partials ⚠️
daemon/api_confdb.go 62.50% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #17709      +/-   ##
==========================================
+ Coverage   78.36%   78.43%   +0.07%     
==========================================
  Files        1417     1410       -7     
  Lines      200507   200852     +345     
  Branches     2503     2503              
==========================================
+ Hits       157129   157548     +419     
+ Misses      33953    33852     -101     
- Partials     9425     9452      +27     
Flag Coverage Δ
unittests 78.43% <71.57%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

confdb confdb work (previously called registries and before aspects) Needs Documentation -auto- Label automatically added which indicates the change needs documentation Run only one system Only runs spread tests on one system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant