ISC Open Source Projects issueshttps://gitlab.isc.org/groups/isc-projects/-/issues2022-11-02T15:08:43Zhttps://gitlab.isc.org/isc-projects/kea/-/issues/47Update network/subnet hooks to handle new classification fields2022-11-02T15:08:43ZGhost UserUpdate network/subnet hooks to handle new classification fields[#5374](https://oldkea.isc.org/ticket/5374) was merged but introduced new features which require an update of hooks managing shared networks and subnets.[#5374](https://oldkea.isc.org/ticket/5374) was merged but introduced new features which require an update of hooks managing shared networks and subnets.backloghttps://gitlab.isc.org/isc-projects/kea/-/issues/44make database config parsing more flexible2022-11-02T15:08:41ZGhost Usermake database config parsing more flexibleCf. #5528 comments (look for "line 125").Cf. #5528 comments (look for "line 125").backloghttps://gitlab.isc.org/isc-projects/kea/-/issues/22stringop-truncation warnings2022-11-02T15:08:41ZFrancis Dupontstringop-truncation warningsG++ 8 has a new warning stringop truncation which is emitted when strncat or strncpy (only the second in kea) fails to terminate (i.e. append a null character) its result.
There are on Fedora 28 spurious warnings on local/unix socket ad...G++ 8 has a new warning stringop truncation which is emitted when strncat or strncpy (only the second in kea) fails to terminate (i.e. append a null character) its result.
There are on Fedora 28 spurious warnings on local/unix socket address or ifname because they are filled using strncpy.
I have a mixed feeling about this: IMHO the issue is not in Kea but in the system header files which should add a ```nonstring``` attribute but did not, so no action is a possible answer to this...backloghttps://gitlab.isc.org/isc-projects/kea/-/issues/3298Make test utility class MemHostDataSource thread-safe2024-03-21T15:02:10ZAndrei Pavelandrei@isc.orgMake test utility class MemHostDataSource thread-safe`MemHostDataSource` is used in certain unit tests.
RADIUS MT unit tests required `MemHostDataSource` to be thread-safe, so the `TestHostCache` that derives it overrode all its methods and added a `lock_guard` to each.
To avoid this boi...`MemHostDataSource` is used in certain unit tests.
RADIUS MT unit tests required `MemHostDataSource` to be thread-safe, so the `TestHostCache` that derives it overrode all its methods and added a `lock_guard` to each.
To avoid this boilerplate code, ideally, `MemHostDataSource` should be made thread-safe itself.
This was not done at the time due to lack of time before the release.
When this is done, remember to remove the overridden methods from `TestHostCache`:
- `premium/src/hooks/dhcp/radius/tests/access_unittests.cc`
- `premium/src/hooks/dhcp/radius/tests/accounting_unittests.cc`
@fdupont says
> Note the mutex must be at most protected.backloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1331Change the protobuf field names to follow the naming convention2024-03-19T14:59:57ZSlawek FigielChange the protobuf field names to follow the naming conventionThe field names should be written using the `snake_case`. We use `camelCase`.
It causes the generated GRPC API files for non-Go languages not to follow the expected naming conventions.
```
message GetStateRsp {
string agentVersion = 1...The field names should be written using the `snake_case`. We use `camelCase`.
It causes the generated GRPC API files for non-Go languages not to follow the expected naming conventions.
```
message GetStateRsp {
string agentVersion = 1;
repeated App apps = 2;
string hostname = 3;
int64 cpus = 4;
string cpusLoad = 5;
int64 memory = 6;
int64 usedMemory = 7;
int64 uptime = 8;
string error = 9;
string os = 10;
string platform = 11;
string platformFamily = 12;
string platformVersion = 13;
string kernelVersion = 14;
string kernelArch = 15;
string virtualizationSystem = 16;
string virtualizationRole = 17;
string hostID = 18;
bool agentUsesHTTPCredentials = 19;
}
```
References: https://protobuf.dev/programming-guides/style/#message-field-namesbackloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1329Event-driven HA status monitoring2024-03-26T14:55:35ZMarcin SiodelskiEvent-driven HA status monitoringCurrently we poll every 10s for the current HA state. I'd like to suggest that we move to an event-based monitoring where the changes will be signaled by the server to the subscribers over SSE. This should reduce the amount of processing...Currently we poll every 10s for the current HA state. I'd like to suggest that we move to an event-based monitoring where the changes will be signaled by the server to the subscribers over SSE. This should reduce the amount of processing in the browser and should cause an immediate reaction to the HA state changes.backloghttps://gitlab.isc.org/isc-projects/kea/-/issues/3275Kea DB allows to store too short identifier in the lease table2024-03-21T14:50:56ZSlawek FigielKea DB allows to store too short identifier in the lease tableWhile performing some experiments in Stork, I found that the Kea database accepts identifiers that are too short (less than 3 bytes) in the `lease6` table. It causes the error to be thrown when the identifier is processed. I noticed it b...While performing some experiments in Stork, I found that the Kea database accepts identifiers that are too short (less than 3 bytes) in the `lease6` table. It causes the error to be thrown when the identifier is processed. I noticed it blocks fetching this lease by API. I don't know if it has any other internal consequences.
Steps to reproduce:
1. Setup Kea 2.3.8 or above with PostgreSQL.
2. Configure lease database.
3. Insert a lease with too short DUID (e.g., `00`)
```
INSERT INTO lease6(address, duid, valid_lifetime, expire, subnet_id, pref_lifetime, lease_type, iaid, prefix_len, hwtype, hwaddr_source, state) VALUES('3001:db8:1::2', DECODE('00', 'hex'), 3600, NOW() + interval '1' MONTH, 1, 1800, 0, 1, 128, 0, 0, 1);
```
4. Send the `lease-get` command with the specified address (i.e., `3001:db8:1::2`).
5. Observe the error:
```
stork-agent-kea6-1 | INFO COMMAND_RECEIVED Received command 'lease6-get'
stork-agent-kea6-1 | INFO CTRL_AGENT_COMMAND_RECEIVED command lease6-get received from remote address 127.0.0.1
stork-agent-kea6-1 | INFO COMMAND_RECEIVED Received command 'lease6-get'
stork-agent-kea6-1 | ERROR LEASE_CMDS_GET6_FAILED lease6-get command failed (parameters: { "ip-address": "3001:db8:1::2", "type": "IA_NA" }, reason: Could not convert data to Lease6, reason: identifier is too short (1), at least 3 is required)
stork-agent-kea6-1 | ERROR HOOKS_CALLOUT_ERROR error returned by callout on hook $lease6_get registered by library with index 1 (callout address 0x7f12e8310e90) (callout duration 0.593 ms)
stork-agent-kea6-1 | INFO CTRL_AGENT_COMMAND_FORWARDED command lease6-get successfully forwarded to the service dhcp6 from remote address 127.0.0.1
```backloghttps://gitlab.isc.org/isc-projects/kea/-/issues/3272Refactor ProcessSpawn2024-03-14T14:34:16ZAndrei Pavelandrei@isc.orgRefactor ProcessSpawnThere are a few things that could be improved in the ProcessSpawn implementation. They become more apparent now that the synchronous functionality has been added to it, but the issues were present before too.
- The asynchronous implemen...There are a few things that could be improved in the ProcessSpawn implementation. They become more apparent now that the synchronous functionality has been added to it, but the issues were present before too.
- The asynchronous implementation of ProcessSpawn relies on having a global IO signal set and on having the IO service being periodically polled in order to wait for child processes which is why it uses the main IO service. For this reason:
- There needs to be a dedicated AsyncProcessSpawn class that should be a singleton to signal to the developer that it has global dependency objects.
- There needs to be a method in AsyncProcessSpawn that sets the IO service. It needs to be callable only once and called as close as possible to the creation of the main IO service. Spawning would throw if the IO service is not initialized. This is to avoid the current behavior which sets the IO service on the first ProcessSpawn creation which could be on a null IOServicePtr. See `src/hooks/dhcp/run_script/run_script.cc` or `src/hooks/dhcp/forensic_log/rotating_file.cc`.
- There should be a new class encapsulating the synchronous implementation, say `SyncProcessSpawn`. It does not have to be a singleton since waiting for the child process is done in the scope it was declared, and the object can be safely deleted afterwards.
- It is worth considering to change the sync variant to use an IO signal set and an IO service like the async variant, but these should not be global, but declarable by the developer, or even better, hidden by the implementation.
- The dismiss feature in spawn is not used in code. I suggest removing it.
- The async process spawn is currently fire-and-forget. It would be nice for the async process spawn to have the ability to be notified that the process has finished and that its status is available. Maybe with the help of a condition variable?backloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1317Refactor creating Kea commands2024-03-05T15:01:41ZSlawek FigielRefactor creating Kea commandsThe Kea commands are created in several places in Stork. We construct them by writing the command name freely and passing the raw dictionary as arguments. It is error-prone and duplicates the code.
We should refactor it. We can define a...The Kea commands are created in several places in Stork. We construct them by writing the command name freely and passing the raw dictionary as arguments. It is error-prone and duplicates the code.
We should refactor it. We can define a dedicated constructor for each command to accept the necessary arguments in a structured form and ensure the command name is correct.
Additionally, the constructor could return the expected response type. Currently, the response definitions are distributed over the application, or we use raw maps to access data.backloghttps://gitlab.isc.org/isc-projects/kea/-/issues/3255CA reconfig doesn't pick up on new certificates2024-02-22T15:00:09ZPeter DaviesCA reconfig doesn't pick up on new certificatesCA reconfig doesn't pick up on new certificates:
After changing the certificates directory in the kea-ctrl-agent configuration
file and sending a SIGHUP signal, kea-ctrl-agent reports
DCTL_CONFIG_COMPLETE server has completed...CA reconfig doesn't pick up on new certificates:
After changing the certificates directory in the kea-ctrl-agent configuration
file and sending a SIGHUP signal, kea-ctrl-agent reports
DCTL_CONFIG_COMPLETE server has completed configuration:
However, the agent still uses the old certificates:
Users that employ the systemd restart command may expect a different behaviour
A workaround would be to kill the process and restart a new one.
[000016830](https://isc.lightning.force.com/lightning/r/Case/500S60000055eAC/view)backloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1315Prometheus exporters should fetch the data on demand2024-03-05T14:56:38ZSlawek FigielPrometheus exporters should fetch the data on demandCurrently, we periodically aggregate the metrics for Prometheus purposes using internal Stork intervals. It means the Prometheus collector always pulls a bit of outdated cached values.
We should avoid using the internal collecting loop ...Currently, we periodically aggregate the metrics for Prometheus purposes using internal Stork intervals. It means the Prometheus collector always pulls a bit of outdated cached values.
We should avoid using the internal collecting loop and our own intervals. The Prometheus collector should be the one to decide when and how often the data are fetched.
- [ ] The Stork agent should send the `statistics-get-all` command and process the response on request to the `/metrics` endpoint
- [ ] The Stork server should retrieve the adjusted utilizations and machine counters on request to the `/metrics` endpointbackloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1314Include the DHCP option names in the hash calculations2024-03-05T14:46:08ZSlawek FigielInclude the DHCP option names in the hash calculationsIn #977, we decided to calculate the DHCP option hash excluding its name, because the lookup for custom DHCP options is not implemented. It causes the DHCP option name to be unknown in several use cases, so we don't acquire an expected h...In #977, we decided to calculate the DHCP option hash excluding its name, because the lookup for custom DHCP options is not implemented. It causes the DHCP option name to be unknown in several use cases, so we don't acquire an expected hash.
The support for the custom DHCP option will be added in #954. After merging it, we should include the DHCP option in the hash calculations and remove the temporary workaround.backloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1309Support for non-default BIND 9 views2024-03-05T14:37:16ZSlawek FigielSupport for non-default BIND 9 viewsThe issue was reported on [our mailing list](https://lists.isc.org/pipermail/stork-dev/2024-February/000049.html):
> the fact that i use views in my bind config is why there is no data
showing up for one host.
The attached statistics:
...The issue was reported on [our mailing list](https://lists.isc.org/pipermail/stork-dev/2024-February/000049.html):
> the fact that i use views in my bind config is why there is no data
showing up for one host.
The attached statistics:
> ```
> ++ Cache Statistics ++
> [View: internal (Cache: internal)]
> 0 cache hits
> 0 cache misses
> 0 cache hits (from query)
> 0 cache misses (from query)
> 0 cache records deleted due to memory exhaustion
> 0 cache records deleted due to TTL expiration
> 0 covering nsec returned
> 0 cache database nodes
> 0 cache NSEC auxiliary database nodes
> XX cache database hash buckets
> XXXXXXXXXX cache tree memory total
> XXXXX cache tree memory in use
> 0 cache tree highest memory in use
> XXXXXXXXX cache heap memory total
> XXXX cache heap memory in use
> 0 cache heap highest memory in use
> [View: _bind (Cache: _bind)]
> 0 cache hits
> 0 cache misses
> 0 cache hits (from query)
> 0 cache misses (from query)
> 0 cache records deleted due to memory exhaustion
> 0 cache records deleted due to TTL expiration
> 0 covering nsec returned
> 0 cache database nodes
> 0 cache NSEC auxiliary database nodes
> XX cache database hash buckets
> XXXXX cache tree memory total
> XXXXX cache tree memory in use
> 0 cache tree highest memory in use
> XXXX cache heap memory total
> XXXX cache heap memory in use
> 0 cache heap highest memry in use
> ```
> there is no _default zone, which might be due to my use of
> views, or different naming convention (_bind vs _default).
>
> is there a way to tell Stork that i want the stats from the views that
> i am running, or global stats from all views?
Unfortunately, Stork has no possibility to change the BIND 9 view name. The `_default` name is hard-coded in several places.backloghttps://gitlab.isc.org/isc-projects/kea/-/issues/3244Allow redefine of standard DHCP options2024-03-07T12:07:26ZDarren AnkneyAllow redefine of standard DHCP optionsCurrently, it isn't possible to redefine any standard DHCP option that is defined in Kea. This is a good practice because it prevents administrators from shooting themselves in the foot. However, DHCPv4 is quite old and there have been...Currently, it isn't possible to redefine any standard DHCP option that is defined in Kea. This is a good practice because it prevents administrators from shooting themselves in the foot. However, DHCPv4 is quite old and there have been some newer RFCs that define options that vendors previously could freely use.
An example of this is an older vendor of IP phones who used option 156 with some of their equipment. Option 156 is part of Bulk Lease Query now ([RFC 6926](https://datatracker.ietf.org/doc/html/rfc6926)) since about 2013. This IP phone manufacturer existed from 1998, however. This option was not defined in ISC DHCP, and so could be used there. The option is defined in Kea, though not presently used. Option 156, in the RFC, is defined as containing an integer as a flag of sorts. The old IP phone vendor needs option 156 to contain a string of some kind. This is only an example. This is probably not a one-off situation.
It is unreasonable to require administrators to purchase new equipment as part of the move to Kea, and therefore, it would be better to, in some way, allow the redefine of standard options. This should probably be a deliberate act somehow, perhaps requiring a keyword of some sort, so that the administrator is aware of what they are doing.
[SF1626](https://isc.lightning.force.com/lightning/r/Case/500S6000004IKcWIAW/view)backloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1308Missing HTTP 404 dedicated page2024-03-05T14:33:42ZSlawek FigielMissing HTTP 404 dedicated pageThe issue was found by @piotrek during 1.15 sanity checks: https://gitlab.isc.org/isc-projects/stork/-/issues/1296#note_434193
It looks like there is no proper 404 handling?
I try to open e.g. `127.0.0.1:8080/random/url` and I get redi...The issue was found by @piotrek during 1.15 sanity checks: https://gitlab.isc.org/isc-projects/stork/-/issues/1296#note_434193
It looks like there is no proper 404 handling?
I try to open e.g. `127.0.0.1:8080/random/url` and I get redirected to home url, but the page body is blank, there is no feedback about redirection/404.
![image](https://gitlab.isc.org/isc-projects/stork/uploads/25290aede3bbe1af07533048cc13a0cd/image.png)backloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1302Add spacing between subnet tab menu and filter box2024-02-13T14:56:20ZSlawek FigielAdd spacing between subnet tab menu and filter boxThe issue was found by @slawek during 1.15 sanity checks: https://gitlab.isc.org/isc-projects/stork/-/issues/1296#note_434170
There is no spacing between the filter box and menu tab on the subnet list:
![image](https://gitlab.isc.org/i...The issue was found by @slawek during 1.15 sanity checks: https://gitlab.isc.org/isc-projects/stork/-/issues/1296#note_434170
There is no spacing between the filter box and menu tab on the subnet list:
![image](https://gitlab.isc.org/isc-projects/stork/uploads/01e35be142883b9ce9856f237341de07/image.png)backloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1300Check the minimum Java version in build system and describe it in docs2024-02-13T14:52:11ZSlawek FigielCheck the minimum Java version in build system and describe it in docsThe issue was found by @marcin during 1.15 sanity checks: https://gitlab.isc.org/isc-projects/stork/-/issues/1296#note_434164
The current version of the OpenAPI generator requires at least OpenJRE 11. The build system checks only if the...The issue was found by @marcin during 1.15 sanity checks: https://gitlab.isc.org/isc-projects/stork/-/issues/1296#note_434164
The current version of the OpenAPI generator requires at least OpenJRE 11. The build system checks only if the `java` binary is present and the documentation specifies that Java is required without providing a minimum version.
We should somehow cover the java requirement checks in the rake files or/and the docs.backloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1298Coherent color scheme2024-02-13T14:50:02ZAndrei Pavelandrei@isc.orgCoherent color schemeStork has a blue-centered color scheme, but occassionally you see a gray background color reminiscent of Windows 95. It looks out of place. The color scheme could be more coherent.Stork has a blue-centered color scheme, but occassionally you see a gray background color reminiscent of Windows 95. It looks out of place. The color scheme could be more coherent.backloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1291QA: Consider secret scanning2024-02-07T11:24:58ZTomek MrugalskiQA: Consider secret scanningTo prevent token and other secrets leaking, we might consider deploying secret scanning tools. Here's two that we might consider:
- [trufflehog](https://github.com/trufflesecurity/trufflehog) -13.2k stars on github
- [gitleaks](https://...To prevent token and other secrets leaking, we might consider deploying secret scanning tools. Here's two that we might consider:
- [trufflehog](https://github.com/trufflesecurity/trufflehog) -13.2k stars on github
- [gitleaks](https://gitleaks.io/) - 14.6k stars on github
There are plenty others, too.backloghttps://gitlab.isc.org/isc-projects/stork/-/issues/1290Hook changelogs in release notes2024-02-06T14:58:17ZSlawek FigielHook changelogs in release notesCurrently, we describe the changes provided in the hooks in the changelog in the main repository. It is pretty convenient because, anyway, we need to update the hook submodule reference to the main repository if any changes were provided...Currently, we describe the changes provided in the hooks in the changelog in the main repository. It is pretty convenient because, anyway, we need to update the hook submodule reference to the main repository if any changes were provided there.
But the hooks contain their own changelogs but they are not used in the release process. In Kea, we present them together with release notes.
@andrei proposed to do the same in Stork in https://gitlab.isc.org/isc-projects/stork/-/merge_requests/716#note_432749 .
Kea has only one repository for all hooks. It means there is only one changelog for all hooks, so in total, there are two changelogs (main and hooks) to maintain. In Stork, we plan to store every hook in a separate repository. Finally, you will have multiple changelogs that need to be processed to prepare the release notes. As far as I know, it is mainly a manual job, right? So, realizing this idea may increase the complexity work of release engineers.
We can extend the release utilities in the main repository to automate including the hook changelogs in the release notes.backlog