Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: fix nondeterministic policy generation #1053

Merged
merged 3 commits into from
Dec 5, 2024
Merged

Conversation

elchead
Copy link
Contributor

@elchead elchead commented Dec 4, 2024

Context

This commit introduced a reproducibility issue for the workload policies.

Ticket: AB#4297

Reproducer

Take a simple example pod and generate it multiple times for K3S-QEMU-TDX (or any upstream Kata platform). The policy and hash will change from time to time.

while true; do
/nix/store/8di8ps56s9acndipiqynak6qilrffl97-contrast-1.2.0-pre-cli/bin/contrast \
    generate --skip-initializer --reference-values K3s-QEMU-TDX test.yaml >/dev/null 2>&1
cat test.yaml | yq '.metadata.annotations["io.katacontainers.config.agent.policy"]' | base64 -d | sha256sum
done

Reason:

> diff 2.rego 3.rego 
1451,1452c1451,1452
<             "net.ipv4.ping_group_range": "0 2147483647",
<             "net.ipv4.ip_unprivileged_port_start": "0"
---
>             "net.ipv4.ip_unprivileged_port_start": "0",
>             "net.ipv4.ping_group_range": "0 2147483647"
1720,1721c1720,1721
<             "net.ipv4.ping_group_range": "0 2147483647",
<             "net.ipv4.ip_unprivileged_port_start": "0"
---
>             "net.ipv4.ip_unprivileged_port_start": "0",
>             "net.ipv4.ping_group_range": "0 2147483647"

The serialization of the map elements is nondeterministic.

Proposed changes

  • use a btreemap instead of a hashmap in the genpolicy tool, because its JSON serialization is deterministic
  • add an e2e test

Additional information

@elchead elchead added the bug fix Fixing a user facing bug label Dec 4, 2024
@elchead elchead force-pushed the as/fix-sysctl-serialize branch 2 times, most recently from fef7d7a to e5a8aa9 Compare December 4, 2024 17:11
@elchead elchead marked this pull request as ready for review December 4, 2024 17:13
@elchead elchead requested review from burgerdev and removed request for burgerdev and katexochen December 4, 2024 17:13
@elchead elchead force-pushed the as/fix-sysctl-serialize branch from e5a8aa9 to 0967628 Compare December 4, 2024 17:17
Copy link
Contributor

@burgerdev burgerdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks a lot!

e2e/policy/deterministic_test.go Outdated Show resolved Hide resolved
@elchead elchead merged commit 5b3a3f8 into main Dec 5, 2024
13 checks passed
@elchead elchead deleted the as/fix-sysctl-serialize branch December 5, 2024 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Fixing a user facing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants