#5584 closed enhancement (complete)

Extend leases storage with user context

Reported by: tomek Owned by: fdupont
Priority: medium Milestone: Kea1.5
Component: database-all Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 10
Total Hours: 10 Internal?: no

Description

During a discussion with a customer it was brought to our attention that the concept of user context would be useful for leases.

The lease structure would be extended with the ability to store user context, with is essentially a JSON. It would be stored as plain text. Kea would not use this data itself, but instead would make it available to hooks and would return it using leaseX-get and other commands.

Subtickets

Change History (10)

comment:1 Changed 17 months ago by tomek

  • Milestone changed from Kea1.x to Kea1.5

comment:2 Changed 17 months ago by tomek

Note we already have user context implemented for global config, shared networks, subnets, pools and host reservations. Especially the host reservations is useful, because these are stored in a database, so it's very similar to leases in that regard.

One particular use case for this would be to store any information needed for IPv6 reconfigure, such as interface name or link-layer client address. The obvious benefit of using user context rather than adding fixed fields is that the schema would be updated just once. If we later discover that additional parameters are needed (e.g. some relay info), we will be able to extend the user context without changing the DB schema. This is important for existing deployments, which dislike schema changes.

Last edited 17 months ago by tomek (previous) (diff)

comment:3 Changed 17 months ago by fdupont

Related to forensic/legal log, first by #5639 (merge schemas) and second because it is natural to append the user context to logs.
So the question is how to organize that, e.g. merge #5639 into this ticket?

comment:4 Changed 17 months ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

comment:5 Changed 17 months ago by fdupont

  • Add Hours to Ticket changed from 0 to 8

Mostly done:

  • schemas updated
  • core code updated
  • unit tests updated
  • system tests (kea--admin) updated
  • forensic/legal log updated
  • premium share directory removed

Compile (including a fix for silly (clearly clang bug) error about toElement overload ambiguity) and pass checks on macOS.

To do:

  • update lease cmd hook
  • update doc
  • create a ticket to apply this to RADIUS accounting

comment:6 Changed 17 months ago by fdupont

  • Add Hours to Ticket changed from 8 to 10
  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing
  • Total Hours changed from 0 to 10

Done. Note there is a second trac5584 branch in premium for forensic/legal log updates.

comment:7 Changed 17 months ago by tomek

  • Owner changed from UnAssigned to tomek

comment:8 follow-up: Changed 17 months ago by tomek

  • Owner changed from tomek to fdupont

I did review the trac5584 in the kea repo. I did not review
the premium branch, because it does not belong to this ticket.


I've made some minor changes. Please pull and review.


This is not a backward compatible change, so the major no minor
versions should be bumped (should be 7.0 for mysql,
and 5.0 for postgres). CQL bump is ok.

Adding logs table in a ticket about user contexts
Logs have nothing to do with this ticket. I really don't like
mixing up two independent tickets. The next time I'll reject
similar changes. But since you already put them in, let's keep
them this time.

src/share/database/scripts/pgsql/dhcpdb_create.pgsql
Why is the user_context dumped in the middle? As a new column, it
should be dumped last.

src/bin/admin/tests/data/pgsql.lease6_dump_test.reference.csv
Why is user_context not added last? The test data doesn't look right.
The user_context should be last.


Ok, managed to run unit tests on all four backends. They passed.
Built on ubuntu 17.10 x64.


After you address the comment about

This requires a ChangeLog entry. Here's my proposal:

14xx.	[func]		fdupont
	Lease objects and lease backends are now able to store user
        context. User context can store an arbitrary data as long
        as it is in JSON format. Database schemas updated.
	(Trac #5649, git cbc29128863916a13364749bf681586aea2aa51e)

comment:9 in reply to: ↑ 8 Changed 16 months ago by fdupont

Replying to tomek:

I did review the trac5584 in the kea repo. I did not review
the premium branch, because it does not belong to this ticket.

=> not fully true but as there is a ticket for premium it does not matter...

I've made some minor changes. Please pull and review.

=> done.

This is not a backward compatible change, so the major no minor
versions should be bumped (should be 7.0 for mysql,
and 5.0 for postgres). CQL bump is ok.

=> it was not clear a major version was required (at the exception
of CQL which does not support minor versions). As I did it for
CQL I know how to do so it is done.

Adding logs table in a ticket about user contexts
Logs have nothing to do with this ticket. I really don't like
mixing up two independent tickets. The next time I'll reject
similar changes. But since you already put them in, let's keep
them this time.

=> my idea was to minimize the number of schema upgrades.
BTW dhcpdb_create.mysql is now very slow so it is not a random
feeling... note I don't say we should address this issue now as it
has impact only on developer, i.e. us.

src/share/database/scripts/pgsql/dhcpdb_create.pgsql
Why is the user_context dumped in the middle? As a new column, it
should be dumped last.

=> no specific reason. Fixed.

src/bin/admin/tests/data/pgsql.lease6_dump_test.reference.csv
Why is user_context not added last? The test data doesn't look right.
The user_context should be last.

=> same issue. Fixed.

After you address the comment about

=> addressed but I need to run again full tests so it will take take.

This requires a ChangeLog entry. Here's my proposal:

14xx.	[func]		fdupont
	Lease objects and lease backends are now able to store user
        context. User context can store an arbitrary data as long
        as it is in JSON format. Database schemas updated.
	(Trac #5649, git cbc29128863916a13364749bf681586aea2aa51e)

=> when it will be merged #5639 will become available.

comment:10 Changed 16 months ago by fdupont

  • Resolution set to complete
  • Status changed from reviewing to closed

Merged. Closing.

Note: See TracTickets for help on using tickets.