Make lease times configurable - #464
Conversation
Prefered- and valid life time are configurable, defaulting to 24h for all plugins Signed-off-by: Damyan Yordanov <damyan.yordanov@sap.com>
github-actions
Bot
added
documentation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds configurable preferred and valid lease lifetimes to all four plugins. It defines shared defaults and validation, wires configuration into plugin setup, updates DHCP responses, and adds examples, documentation, and tests. ChangesLease lifetime configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant StatelessPlugin
participant loadConfig
participant YAMLFile
participant LeaseTimes
participant DHCPv6Handler
StatelessPlugin->>loadConfig: Load optional configuration path
loadConfig->>YAMLFile: Read and parse YAML
loadConfig->>LeaseTimes: Validate and resolve lifetimes
StatelessPlugin->>DHCPv6Handler: Create handler after configuration succeeds
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
plugins/stateless/plugin_test.go (1)
90-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify custom lifetimes in the DHCP response.
This test only checks
preferredLifeTimeandvalidLifeTime. Callhandler6afterloadConfig(cfgPath)and assert the returnedOptIAAddresshas1hand2h. A handler that still emits fixed lifetimes currently passes this test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/stateless/plugin_test.go` around lines 90 - 103, Extend TestHandler6_ConfigurableLeaseTimes after loadConfig to invoke handler6 with the test’s DHCP request context, then inspect the returned OptIAAddress and assert its preferred and valid lifetimes are one hour and two hours respectively, while retaining the existing configuration-variable checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/api/leasetime_config.go`:
- Around line 41-45: Update LeaseTimes.Validate to reject either resolved
lifetime when preferred or valid is negative, returning a validation error
before the existing preferred > valid ordering check; retain the current
ordering validation for non-negative values.
In `@plugins/stateless/plugin.go`:
- Around line 34-37: Update setup6 and handler6 so lease durations are
instance-scoped rather than stored in the package-level preferredLifeTime and
validLifeTime variables. Resolve the durations in setup6 via loadConfig, then
return a handler closure capturing those values so each handler uses the
configuration from its own setup invocation.
- Around line 77-79: Update the validation flow around
config.LeaseTimes.Validate to reject resolved preferred and valid lifetimes that
are negative or exceed the uint32-second range used by dhcpv6.Duration
serialization, before accepting the configuration. Preserve existing lease
relationship validation, and add coverage for both negative lifetimes and values
above the maximum representable duration.
In `@README.md`:
- Line 279: Update the Stateless description to use the hyphenated term
“non-temporary IPv6 addresses” in place of “non temporary IPv6 addresses,”
without changing the surrounding documentation.
- Around line 120-125: Update the OnMetal configuration description near the
leaseTimes example so it no longer says the configuration contains only the
prefix delegation length. Mention both configurable leaseTimes
settings—preferred and valid lifetimes—alongside the prefix delegation length,
while preserving the existing YAML example.
---
Nitpick comments:
In `@plugins/stateless/plugin_test.go`:
- Around line 90-103: Extend TestHandler6_ConfigurableLeaseTimes after
loadConfig to invoke handler6 with the test’s DHCP request context, then inspect
the returned OptIAAddress and assert its preferred and valid lifetimes are one
hour and two hours respectively, while retaining the existing
configuration-variable checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c15c1fc8-abb6-4c1f-9ec0-e490e3481737
📒 Files selected for processing (16)
README.mdexample/bluefield_config.yamlexample/onmetal_config.yamlexample/oob_config.yamlexample/stateless_config.yamlinternal/api/bluefield_config.gointernal/api/leasetime_config.gointernal/api/onmetal_config.gointernal/api/oob_config.gointernal/api/stateless_config.goplugins/bluefield/plugin.goplugins/bluefield/plugin_test.goplugins/onmetal/plugin.goplugins/oob/plugin.goplugins/stateless/plugin.goplugins/stateless/plugin_test.go
| var ( | ||
| preferredLifeTime time.Duration | ||
| validLifeTime time.Duration | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect Setup6 to be single-shot and serialized, or verify that handler state is instance-scoped.
rg -n -C 8 'Setup6\s*:|Setup6\s*\(|setup6\s*\(' --glob '*.go' .
rg -n -C 6 '\bpreferredLifeTime\b|\bvalidLifeTime\b|\bhandler6\b' plugins/stateless --glob '*.go'Repository: ironcore-dev/FeDHCP
Length of output: 44821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- plugin.go outline ---\n'
ast-grep outline plugins/stateless/plugin.go || true
printf '\n--- plugins/stateless/plugin.go ---\n'
cat -n plugins/stateless/plugin.go
printf '\n--- registration / server lifecycle references ---\n'
rg -n -C 5 '\.Setup(4|6)\(|Setup(4|6)\(|LoadPlugin|coredhcp|Server|NewRequest|plugin\.Plugin|Handler(4|6)' --glob '*.go' .Repository: ironcore-dev/FeDHCP
Length of output: 248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- plugins/stateless/plugin.go ---'
cat -n plugins/stateless/plugin.go
printf '%s\n' '--- coredhcp / plugin registration references ---'
rg -n -C 5 '\.Setup(4|6)\(|Setup(4|6)\(|LoadPlugin|coredhcp|Server|NewRequest|plugin\.Plugin|Handler(4|6)' --glob '*.go' .Repository: ironcore-dev/FeDHCP
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- core DHCP module versions ---'
rg -n 'github.com/coredhcp/coredhcp' go.mod go.sum | head -40
printf '%s\n' '--- main plugin wiring ---'
cat -n main.go | sed -n '1,140p'
printf '%s\n' '--- core DHCP config/server/plugin setup definitions ---'
go mod download github.com/coredhcp/coredhcp 2>/dev/null || true
MODULE_CACHE="$(go env GOMODCACHE 2>/dev/null || true)"
if [ -n "$MODULE_CACHE" ]; then
find "$MODULE_CACHE/github.com/coredhcp/coredhcp@*" -path '*coredhcp/config*' -o -path '*coredhcp/server*' -o -path '*coredhcp/plugins*' | head -80
fiRepository: ironcore-dev/FeDHCP
Length of output: 5582
🌐 Web query:
coredhcp config server Start register plugins LoadConfig LoadModule handlers
💡 Result:
CoreDHCP is a modular DHCP server where core functionality is implemented via a plugin architecture [1][2]. Because Go is a compiled language without native dynamic loading, plugins must be explicitly registered and compiled into the server binary rather than being loaded at runtime [3]. Configuration and Plugin Lifecycle: 1. Building with Plugins: To include specific plugins, you must use the coredhcp-generator tool [3]. This tool generates a custom main.go file that imports and registers the desired plugins [3]. You then compile this generated source code to produce your custom CoreDHCP binary [3]. 2. Server Configuration: The behavior of the server is defined in a configuration file (typically config.yml) [4][1]. The plugins section is mandatory and defines which plugins are active and their associated arguments [4]. 3. Plugin Execution Order: The order of plugins in the config.yml file is significant [1][5]. When a DHCP request arrives, CoreDHCP evaluates plugins sequentially in the order they are listed [1]. This evaluation continues until a plugin responds to or explicitly drops the request [1]. 4. Registration and Loading: When the server starts, it registers and loads the plugins that were compiled into the binary [1][2]. Logs will indicate the registration and loading process of each plugin, such as INFO messages showing "Registering plugin" and "Loading plugin" [1][6]. For detailed examples of configuration syntax and argument formats, refer to the config.yml.example file located in the cmds/coredhcp/ directory of the official repository [4][2].
Citations:
- 1: https://coredhcp.io/
- 2: https://github.com/coredhcp/coredhcp
- 3: https://coredhcp.io/cmds/coredhcp-generator/
- 4: https://github.com/coredhcp/coredhcp/blob/master/cmds/coredhcp/config.yml.example
- 5: Documentation: example config files have wrong order coredhcp/coredhcp#254
- 6: https://coredhcp.io/README.md
🌐 Web query:
CoreDHCP 0.2 LoadModule plugin name args setup6 Handler4 Handler6
💡 Result:
In CoreDHCP, plugins are defined in the configuration file (typically config.yml) under a plugins section, which is a list of maps matching plugin names to their arguments [1]. The architecture does not use a LoadModule directive; instead, plugins are statically linked into the server binary during the build process [2][3][4]. To include and use specific plugins, you must: 1. Build the binary with the desired plugins using the coredhcp-generator tool [2][3]. 2. Configure them in your config.yml file by adding them to the plugins list, where the order of declaration determines the order of execution for incoming requests [1][2][5]. Regarding plugin structure and handlers: Each plugin typically exposes a structure (often named Plugin) that contains setup functions for DHCPv4 and DHCPv6, such as Setup4 and Setup6 [6]. - Setup4 and Setup6: These functions take arguments from the configuration file as a slice of strings (args...string) and return their respective handlers (Handler4 or Handler6) [6]. - Handler4 and Handler6: These are the core functions that process DHCP packets [6][7]. They take the incoming packet and the current state as input and return the modified response packet along with a boolean that indicates whether to stop the evaluation chain [5][7]. If a handler returns a stop value of true, subsequent plugins in the chain are not executed [2][5][7]. Plugin arguments are passed directly from the configuration file list to the setup functions during server initialization [1][6]. External plugins follow this same pattern and are expected to document their specific argument requirements in their own documentation [1].
Citations:
- 1: https://github.com/coredhcp/coredhcp/blob/master/cmds/coredhcp/config.yml.example
- 2: https://coredhcp.io/
- 3: https://coredhcp.io/README.md
- 4: https://github.com/coredhcp/coredhcp
- 5: Documentation: example config files have wrong order coredhcp/coredhcp#254
- 6: https://github.com/coredhcp/plugins/blob/master/redis/plugin.go
- 7: https://github.com/coredhcp/coredhcp/blob/master/server/handle.go
Make lease times instance-scoped
setup6 writes global preferredLifeTime / validLifeTime, and handler6 reads them for every request. If Setup6 is called more than once, later config overwrites the values used by handlers created by earlier configuration. Return a handler closure that captures the resolved durations from loadConfig.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/stateless/plugin.go` around lines 34 - 37, Update setup6 and handler6
so lease durations are instance-scoped rather than stored in the package-level
preferredLifeTime and validLifeTime variables. Resolve the durations in setup6
via loadConfig, then return a handler closure capturing those values so each
handler uses the configuration from its own setup invocation.
| if err := config.LeaseTimes.Validate(); err != nil { | ||
| return fmt.Errorf("invalid lease times: %v", err) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching plugin.go:"
fd -a 'plugin\.go$' . | sed 's#^\./##' | head -20
echo
echo "Relevant file outline/lines:"
if [ -f plugins/stateless/plugin.go ]; then
wc -l plugins/stateless/plugin.go
sed -n '1,220p' plugins/stateless/plugin.go | cat -n
fi
echo
echo "Search for LeaseTimes definition and Validate implementation:"
rg -n "type LeaseTimes|func .*Validate|preferredLifetime|validLifetime|OptIAAddress|Resolve" -S .Repository: ironcore-dev/FeDHCP
Length of output: 13699
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Leasetime config implementation:"
wc -l internal/api/leasetime_config.go
cat -n internal/api/leasetime_config.go
echo
echo "dhcpv6 dependency/version references:"
for f in go.mod go.sum; do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n "insomniacslk/dhcp|dhcpv6" "$f" || true
fi
done
echo
echo "Go tool availability:"
command -v go || true
go version 2>/dev/null || trueRepository: ironcore-dev/FeDHCP
Length of output: 2285
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Go module cache location:"
go env GOPATH GOMODCACHE 2>/dev/null || true
GOAMD64_VARIANT=${GOAMD64_VARIANT:-v1} GO111MODULE=on GOPROXY=https://proxy.golang.org,direct GOSUMDB=sum.golang.org \
go mod download github.com/insomniacslk/dhcp@v0.0.0-20251020182700-175e84fbb167 2>/tmp/gomod.log
echo "Download/module status:"
while IFS= read -r line; do
case "$line" in *\ Error*|*\ warning*|*\ replaced\ module*) echo "$line" ;; esac
done </tmp/gomod.log
[ -s /tmp/gomod.log ] && echo "--- logfile ---" && cat /tmp/gomod.log
echo
echo "Locate installed module and relevant sources:"
MOD=$(go env GOMODCACHE 2>/dev/null)/github.com/insomniacslk/dhcp@v0.0.0-20251020182700-175e84fbb167
if [ -d "$MOD" ]; then
echo "found $MOD"
fd -a '.*\.(go)$' "$MOD" | sed "s#$MOD/##" | head -80
echo "--- OptIAAddress definitions/usages ---"
rg -n "OptIAAddress|PreferredLifetime|ValidLifetime|LeaseLifetime|lifetime" "$MOD" -S
else
echo "not found"
fi
echo
echo "Read source around OptIAAddress lifecycle fields, if present:"
for f in $(rg -l "type OptIAAddress|PreferredLifetime|ValidLifetime|LeaseLifetime" "$MOD" -S); do
echo "--- $f ---"
rg -n -C 8 "type OptIAAddress|PreferredLifetime|ValidLifetime|LeaseLifetime" "$f"
done
echo
echo "Behavioral probe of LeaseTimes.Resolve/Validate for negative inputs:"
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat > "$tmpdir/main.go" >/dev/null <<'EOF'
package main
import (
"fmt"
"time"
)
func main() {
for _, pref := range []time.Duration{-1 * time.Hour, -time.Nanosecond, 0, time.Hour} {
for _, valid := range []time.Duration{-1 * time.Hour, -time.Nanosecond, 0, time.Hour} {
preferred := pref
if preferred == 0 {
preferred = 24 * time.Hour
}
v := valid
if v == 0 {
v = 24 * time.Hour
}
err := "nil"
if preferred > v {
err = fmt.Errorf("preferredLifetime (%s) must not exceed validLifetime (%s)", preferred, v).Error()
}
fmt.Printf("pref=%v valid=%v resolved=(%s,%s) err=%v\n", pref, valid, preferred, v, err)
}
}
}
EOF
(cd "$tmpdir" && go run main.go)
echo
echo "Overflow/maximum duration probe:"
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat > "$tmpdir/main.go" >/dev/null <<'EOF'
package main
import (
"fmt"
"math"
"time"
)
func main() {
max := time.Duration(math.MaxInt64)
fmt.Printf("max duration: %v nanoseconds=%v\n", max, max.Nanoseconds())
for _, sign := range []time.Duration{1, -1} {
for _, d := range []time.Duration{max - 1, max, max + 1, max + time.Hour, 100 * max} {
fmt.Printf("sign=%s d=%v nanoseconds=%v\n", sign, sign*d, sign*d.Nanoseconds())
}
}
fmt.Printf("preferredLifetime: -1h validLifetime: 1h passes Resolve=%q Validate=%q\n", -time.Hour, time.Hour, "")
}
EOF
(cd "$tmpdir" && go run main.go)Repository: ironcore-dev/FeDHCP
Length of output: 2571
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect OptIAAddress implementation:"
rg -n -C 8 "type OptIAAddress|type OptIAAddress |func .*OptIAAddress|PreferredLifetime|ValidLifetime|LeaseLifetime|NewOptIAAddress|Marshal6|Unmarshal6" \
/home/jailuser/go/pkg/mod/github.com/insomniacslk/dhcp@*/dhcpv6 /home/jailuser/go/pkg/mod/github.com/damyan/dhcp@*/dhcpv6 2>/tmp/rg.log || true
[ -s /tmp/rg.log ] && echo "--- rg log ---" && cat /tmp/rg.log
echo
echo "Read candidate OptIAAddress files if found:"
for f in $(rg -l "type OptIAAddress|func .*OptIAAddress" /home/jailuser/go/pkg/mod/github.com/insomniacslk/dhcp@*/dhcpv6 /home/jailuser/go/pkg/mod/github.com/damyan/dhcp@*/dhcpv6 2>/dev/null); do
echo "--- $f ---"
wc -l "$f"
sed -n '1,240p' "$f" | cat -n
done
echo
echo "Stand-alone behavioral model for current LeaseTimes code:"
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat > "$tmpdir/main.go" <<'EOF'
package main
import (
"fmt"
"time"
)
func default(duration time.Duration) time.Duration {
if duration == 0 {
return 24 * time.Hour
}
return duration
}
func resolve(preferred, valid time.Duration) (time.Duration, time.Duration) {
return default(preferred), default(valid)
}
func validate(preferred, valid time.Duration) error {
if preferred > valid {
return fmt.Errorf("preferredLifetime (%s) must not exceed validLifetime (%s)", preferred, valid)
}
return nil
}
func main() {
type Case struct {
preferred time.Duration
valid time.Duration
}
for _, c := range []Case{
{-1 * time.Hour, 1 * time.Hour},
{-time.Nanosecond, time.Hour},
{0, -2 * time.Hour},
{-1 * time.Hour, -2 * time.Hour},
{time.Hour, 2 * time.Hour},
{time.Hour, 0},
} {
p, v := resolve(c.preferred, c.valid)
fmt.Printf("input=(%021d,%021d) -> resolved=(%021d,%021d) validate=%v\n",
c.preferred.Nanoseconds(), c.valid.Nanoseconds(),
p.Nanoseconds(), v.Nanoseconds(),
validate(p, v))
}
}
EOF
go run "$tmpdir/main.go"Repository: ironcore-dev/FeDHCP
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Read Duration implementation:"
for f in $(rg -l "type Duration|func .*Duration.*Marshal|func .*Duration.*Unmarshal|func .*Duration.*ToBytes" /home/jailuser/go/pkg/mod/github.com/insomniacslk/dhcp@*/dhcpv6 /home/jailuser/go/pkg/mod/github.com/damyan/dhcp@*/dhcpv6 2>/tmp/rg2.log); do
echo "--- $f ---"
rg -n -C 6 "type Duration|func .*Duration|ToBytes|Unmarshal|Marshal6|Marshal" "$f"
done
echo
echo "Read duration source:"
for f in $(rg -l "type Duration" /home/jailuser/go/pkg/mod/github.com/insomniacslk/dhcp@*/dhcpv6 /home/jailuser/go/pkg/mod/github.com/damyan/dhcp@*/dhcpv6 2>/tmp/rg3.log); do
echo "--- $f ---"
cat -n "$f"
done
echo
echo "Read first 220 lines of OptIAAddress serialization:"
for f in /home/jailuser/go/pkg/mod/github.com/insomniacslk/dhcp@*/dhcpv6/option_iaaddress.go /home/jailuser/go/pkg/mod/github.com/damyan/dhcp@*/dhcpv6/option_iaaddress.go 2>/dev/null; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f" | sed -n '1,220p'
fi
doneRepository: ironcore-dev/FeDHCP
Length of output: 6154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Read OptIAAddress serialization file:"
cat -n /home/jailuser/go/pkg/mod/github.com/insomniacslk/dhcp@v0.0.0-20251020182700-175e84fbb167/dhcpv6/option_iaaddress.go | sed -n '1,140p'
echo
echo "Deduce Duration overflow behavior from source:"
python3 - <<'PY'
from pathlib import Path
import re
src = Path("/home/jailuser/go/pkg/mod/github.com/insomniacslk/dhcp@v0.0.0-20251020182700-175e84fbb167/dhcpv6/option_nontemporaryaddress.go").read_text()
m = re.search(r'func \(d Duration\) Marshal\(buf \*uio\.Lexer\) \{(?P<body>.*?)\n\}', src, re.S)
if m:
print(m.group(0))
print("contains unsigned conversion:", "uint32" in m.group("body"))
PY
echo
echo "Reproduce current validation logic and max representable duration:"
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat > "$tmpdir/main.go" <<'EOF'
package main
import (
"fmt"
"math"
"time"
)
func main32(v64 int64) uint32 {
if v64 < 0 {
return uint32(v64)
}
if v64 > math.MaxUint32 {
return uint32(math.MaxUint32)
}
return uint32(v64)
}
func main() {
fmt.Printf("Max uint32 seconds duration: %v\n", time.Duration(uint32(math.MaxUint32))*time.Second)
for _, pref := range []string{"-1h", "-59m60s", "0", "4294967295s", "4294967296s"} {
p, _ := time.ParseDuration(pref)
for _, val := range []string{"1h", "-1h", "4294967295s", "4294967296s"} {
v, _ := time.ParseDuration(val)
fmt.Printf("pref=%22s val=%22s round_seconds_pref=%22d round_seconds_val=%22d serialized_pref=%22d serialized_val=%22d passes=%v\n",
pref, val,
p.Round(time.Second)/time.Second,
v.Round(time.Second)/time.Second,
// simulate uint32 cast before write32 and negative two's-complement uint32 semantics
uint32(p.Round(time.Second)/time.Second),
uint32(v.Round(time.Second)/time.Second),
p > v)
}
}
}
EOF
go run "$tmpdir/main.go"Repository: ironcore-dev/FeDHCP
Length of output: 7971
Reject negative and overflowed DHCPv6 lifetimes before validation.
LeaseTimes.Validate() calls Resolve(), which keeps negative durations unchanged, so a config like preferredLifetime: -1h and validLifetime: 1h can pass. At the same time, the dhcpv6.Duration serializer writes uint32(seconds), which wraps both negative values and durations over ~1193046h28m15s. Add bounds checking on resolved values and tests for negative and overflowed lifetimes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/stateless/plugin.go` around lines 77 - 79, Update the validation flow
around config.LeaseTimes.Validate to reject resolved preferred and valid
lifetimes that are negative or exceed the uint32-second range used by
dhcpv6.Duration serialization, before accepting the configuration. Preserve
existing lease relationship validation, and add coverage for both negative
lifetimes and values above the maximum representable duration.
| The leased address preferred and valid lifetimes default to 24 hours and are optionally configurable: | ||
| ```yaml | ||
| leaseTimes: | ||
| preferredLifetime: 24h | ||
| validLifetime: 24h | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the OnMetal configuration description.
Line 114 still says that the configuration contains only the prefix delegation length. The new leaseTimes settings make that statement incorrect. Update the description to mention both settings.
Proposed wording
- The onmetal configuration consists of the prefix delegation length only.
+ The OnMetal configuration consists of the prefix delegation length and optional lease times.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 120 - 125, Update the OnMetal configuration
description near the leaseTimes example so it no longer says the configuration
contains only the prefix delegation length. Mention both configurable leaseTimes
settings—preferred and valid lifetimes—alongside the prefix delegation length,
while preserving the existing YAML example.
| For example, given a link address of `2001:db8:1111:2222:3333::` and a MAC of `aa:bb:cc:dd:ee:ff`, the resulting address is `2001:db8:1111:2222:3333:aabb:ccdd:eeff`. | ||
|
|
||
| Addresses are leased as [non temporary IPv6 addresses](https://datatracker.ietf.org/doc/html/rfc8415#section-6.2) with a 24-hour lifetime. | ||
| Addresses are leased as [non temporary IPv6 addresses](https://datatracker.ietf.org/doc/html/rfc8415#section-6.2). The preferred and valid lifetimes default to 24 hours and are configurable via an optional config file (`stateless_config.yaml`): |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use “non-temporary” in the Stateless description.
Replace “non temporary IPv6 addresses” with “non-temporary IPv6 addresses”.
🧰 Tools
🪛 LanguageTool
[grammar] ~279-~279: Use a hyphen to join words.
Context: ...cdd:eeff`. Addresses are leased as [non temporary IPv6 addresses](https://datatr...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 279, Update the Stateless description to use the
hyphenated term “non-temporary IPv6 addresses” in place of “non temporary IPv6
addresses,” without changing the surrounding documentation.
Source: Linters/SAST tools
mkalcok
left a comment
mkalcok
left a comment
There was a problem hiding this comment.
Hi @damyan. I'm +1 on this feature. It adds a nice level of flexibility that's going to help us with control plane deployments.
Aside from the comments from CodeRabbit about validating negative durations, I left one comment about the change of default behavior for bluefield plugin.
After that is addressed, Im +1 on this change, though I think it'd be good to get +1 also from someone more familiar with this repo.
|
@mkalcok I do not intend to implement that negative-durability-check; I leave to the operator to specify meaningful values. WDYT? |
mkalcok
left a comment
mkalcok
left a comment
There was a problem hiding this comment.
@mkalcok I do not intend to implement that negative-durability-check; I leave to the operator to specify meaningful values. WDYT?
I would personally implement it, just because it's very simple check, but at the same time, I'm not gonna hold up the merge over that. It is a very obvious configuration error of someone puts a negative value in the config.
As per review, check if both lease times are not negative. Zero value still triggers the default of 24H. Signed-off-by: Damyan Yordanov <damyan.yordanov@sap.com>
Implemented. |
2 participants
Prefered- and valid life time are configurable, defaulting to 24h for all plugins
Fixes #463
/cc @mkalcok @Giluerre
Summary by CodeRabbit
New Features
Bug Fixes
Documentation