chore: deprecate old profiletypes in favor of containerprofiles - #897
chore: deprecate old profiletypes in favor of containerprofiles#897matthyx wants to merge 43 commits into
Conversation
… -> ContainerProfile ConvertUserProfilesToContainerProfile produces the single user-defined ContainerProfile equivalent to a legacy user-authored ApplicationProfile + NetworkNeighborhood pair, reusing the existing projectUserProfiles merge onto an empty base. The differential oracle pins the migration contract at the enforcement level: for representative user-defined shapes (opens/exec argv wildcards, HTTP endpoints, egress/ingress + LabelSelector) the ProjectedContainerProfile from the legacy AP+NN overlay path equals the one from using the converted ContainerProfile as the base with no overlay. Behaviour-preserving by construction. Signed-off-by: entlein <einentlein@gmail.com>
…e authoritative base When the user-defined-profile pod label names a ContainerProfile carrying managed-by: User, the cache now uses it directly as the container's base profile (the migrated "new way"), instead of overlaying a legacy ApplicationProfile + NetworkNeighborhood pair. It falls back to the legacy AP+NN pair when no such CP exists, which still fires the existing deprecation signal. - add-time (tryPopulateEntry) and refresh (reconciler) both prefer the user CP, gated on the managed-by: User annotation so a learned CP at the same name is never mistaken for a user-defined one - UserCPRef/UserCPRV bookkeeping mirrors the legacy UserAPRV/UserNNRV RV tracking so the reconciler re-fetches and rebuilds only when the user CP changes Signed-off-by: entlein <einentlein@gmail.com>
Test_28 now creates one ContainerProfile (managed-by: User) carrying the merged exec/syscall + egress/selector surfaces, instead of a separate user-authored ApplicationProfile + NetworkNeighborhood pair — exercising the migrated read-path end to end. Assertions unchanged. Signed-off-by: entlein <einentlein@gmail.com>
…verlay The new path (converted CP as base, no overlay) is ~35% faster and allocates ~27% less than the legacy AP+NN overlay per projection, since it skips the two-object merge. Signed-off-by: entlein <einentlein@gmail.com>
Test_27 (opens R0002, both the regex and the curl-wildcard profile sites), Test_32 (R0040 argv wildcards), and Test_33 (opens wildcard anchoring — the previously-fixed one) now each create a single user-defined ContainerProfile (managed-by: User) carrying the merged exec/open/syscall (+ egress/selector for 32) surfaces, replacing the legacy ApplicationProfile + NetworkNeighborhood pair. Assertions unchanged. Test_28 was ported in an earlier commit. Signed-off-by: entlein <einentlein@gmail.com>
containsDynamicSegment recognised only the one-segment DynamicIdentifier
('⋯'), so a path-surface opens entry bearing the zero-or-more
WildcardIdentifier ('*') — e.g. '/etc/ssl/*' — was routed to Values as if it
were a literal. was_path_opened tolerates this (Values and Patterns are both
matched via CompareDynamic), but it is wrong for any consumer that treats
Values as exact membership, and it drops '*'-only entries a rule needs when
spec.All is false and no prefix/suffix matcher retains them. Recognise both
wildcard markers. Regression test pins '/etc/ssl/*' -> Patterns.
Pre-existing in upstream (identical containsDynamicSegment).
Signed-off-by: Entlein <eineintlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Test_33's anchoring subtests assert R0002 alerts, but the rule's file-access monitoring is opt-in (monitored prefixes incl. /etc/). Test_33 never applied r0002-files-access-enabled.yaml (Test_27 does via enableR0002ForTest), so R0002 never evaluated the opens and every 'expect alert' case silently passed as a no-alert — invisible because Test_33 had never run in CI. Enable it like Test_27. Verified on a live cluster: /etc/ssl/* correctly alerts on the bare parent /etc/ssl and stays silent on the child /etc/ssl/openssl.cnf. Signed-off-by: entlein <einentlein@gmail.com>
…tations A user-authored profile is authoritative and complete by definition — it should not carry the learning-lifecycle status/completion markers, nor a managed-by annotation. The pod's user-defined-profile label is what declares it user-authored (a signature, added by the signing tooling, is the integrity marker). - read-path/reconciler no longer gate on managed-by: the label-referenced CP is used as authoritative, and the entry State is forced to Completed+Full so the rule engine enforces it despite the absent completion annotation - the converter emits clean CPs (name + namespace + spec only) - fake client models an absent overlay-name fetch (drives the legacy fallback) Signed-off-by: entlein <einentlein@gmail.com>
…mple - Test_28 now loads its user-defined ContainerProfile from a real yaml (resources/containerprofile-user-defined-network.yaml) via a loader helper, so the fixture doubles as the copy-pasteable "how to author a user-defined profile" example. - The example — and all the test CPs — carry only name + spec: the nonsensical learning-lifecycle annotations (status/completion) and the managed-by marker are dropped (node-agent now treats a label-referenced CP as authoritative and forces the enforce-state itself). Signed-off-by: entlein <einentlein@gmail.com>
Migrate the network-wildcards NetworkNeighborhood fixtures into their user-authored ContainerProfile form so users can copy-paste them to author user-defined-profile allow-lists: - per-container spec.egress/ingress (NN's spec.containers[] collapses, one CP document per container; fixture 20 splits into two) - no lifecycle annotations — name only (namespace injected by tooling), matching the clean user-managed CP contract - teaching comments preserved verbatim from the NN fixtures Adds 00-fusioncore-homoglyph-attack.yaml: a pinned single-vendor allow-list and the look-alike (homoglyph) domains its exact-match dnsNames compare rejects (each fires R0005). All 21 documents strict-parse against v1beta1 ContainerProfile and carry zero annotations (verified). Signed-off-by: entlein <einentlein@gmail.com>
The component tests build their user-defined profiles inline as ContainerProfiles (Test_27/32/33) or load a CP yaml (Test_28); the legacy ApplicationProfile / NetworkNeighborhood fixtures they were derived from are no longer referenced by any Go test. Migrate the two CT-relevant ones to their user-authored CP form and delete the orphan: - exec-arg-wildcards-profile.yaml (AP curl-32-overlay) -> containerprofile-exec-arg-wildcards.yaml. CP form mirrors Test_32's inline CP exactly (same execs incl. busybox-symlink + literal-* entries, same syscalls, matchLabels app: curl-32). Demonstrates the exec-arg wildcard grammar for authoring. - known-network-neighborhood.yaml (NN fusioncore-network) -> containerprofile-fusioncore-network.yaml. Clean user-managed CP: name only, no managed-by / status / completion annotations. - user-profile.yaml: deleted. Zero references anywhere; its nginx/server exec surface matches no current test or deployment. Both new documents strict-parse against v1beta1 ContainerProfile and carry zero annotations (verified). CT compiles unchanged (go vet -tags component); no Go test referenced the deleted files. Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
… decommission them -step 1 of many -- I understand that this has a long tail of decommissions Signed-off-by: entlein <einentlein@gmail.com>
…storage etc Signed-off-by: entlein <einentlein@gmail.com>
…icontainer labels, tests not reviewed yet Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Co-authored-by: Matthias Bertschy <matthias.bertschy@gmail.com> Signed-off-by: Duck <70207455+entlein@users.noreply.github.com>
Signed-off-by: entlein <einentlein@gmail.com>
Apply had assertion-style unit tests but no frozen contract for its full projected output. Add TestApply_Golden: a corpus (execs incl. literal-* and ellipsis args, opens incl. trailing-* and dynamic segments, syscalls, capabilities, endpoints, ingress/egress with CIDR/*-sentinel/DNS, call stacks) projected via the real CompileSpec, frozen to goldens (regenerate with UPDATE_GOLDEN=1). Plus idempotency and spec-hash-stability invariants. This is a characterization/regression freeze, not a two-path differential -- the legacy AP/NN projection was removed, so there is no second implementation to diff. Signed-off-by: entlein <einentlein@gmail.com>
The projection golden corpus set the deprecated DNS/IPAddress singulars (to distinct values) alongside the modern lists. Drop them: the corpus now uses DNSNames/IPAddresses only (still covering DNS name, literal IP, CIDR and the * sentinel). Goldens regenerated from the CP-native corpus. Signed-off-by: entlein <einentlein@gmail.com>
…ace) The rulemanager CEL libraries already read ContainerProfile data but still carried AP/NN naming. Rename applicationprofile -> containerprofile and networkneighborhood -> containerprofilenetwork (dirs via git mv, packages, types, constructors, LibraryName), and unify the CEL function namespace: ap.* and nn.* -> cp.* (cp.was_executed, cp.is_domain_in_egress, ...). Update cel.go registration, the bundled default-rules.yaml, and the network-wildcards test rules to the cp.* namespace so rules compile against the renamed engine. Rule content mirrors the rulelibrary migrate/sbob rename. Signed-off-by: entlein <einentlein@gmail.com>
The CP-form fixtures lived in a parallel network-wildcards-cp/ dir alongside the now-dead network-wildcards/. 'cp' is an implementation detail, not part of what the fixtures are. Remove the dead dir, move the CP fixtures to network-wildcards/ (update the nnlint/fixtures test references), and drop the explanatory README. Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Resolve the Test_35 collision: upstream added Test_35_ExecTTYFieldTest at the same location as our Test_35_MultiContainerPerContainerBinding. Keep both; renumber ours to Test_36_MultiContainerPerContainerBinding.
Add cluster-free unit coverage for the migration's rule-side surface: - cp.* HTTP evaluators (http.go, was 0%/no file): endpoint/host match, method and prefix/suffix variants, empty-CP, nil-cache error. - isExecInPodSpec + the wasExecuted/wasExecutedWithArgs podspec-exempt fall-through. - containerprofilenetwork matchers: no-match / wildcard-domain / empty-neighborhood. - Declarations smoke+drift guard: compile every cp.* overload, assert the decl set. - profilehelper GetProjectedContainerProfile/GetPodSpec/GetContainerName. - HasFinalApplicationProfile enforce-vs-learn gate (Completed+Full -> enforce). containerprofile 37->69%, containerprofilenetwork 75->80%, profilehelper 95%, HasFinalApplicationProfile 100%. Test-only; no production changes. Signed-off-by: entlein <einentlein@gmail.com>
… racy learning) Test_20/21 hung to the 20-minute global panic in WaitForContainerProfileCompletion*: Test_20's blacklist wait can never succeed (the old completed profile stays matched and blacklisted after the deployment restart), and Test_21's multi-container profile never reached 'completed'. Both raced the natural learn -> daemonset-restart -> deployment-restart -> re-learn cycle. Rewrite to enforce an AUTHORED user-defined ContainerProfile, updated in place: an action not in the profile alerts; the same action, once added, does not. Determinism comes from a positive reload gate -- the update that adds the subject also removes a canary, so the canary starts alerting the moment node-agent reloads the revision (alert-appears signal), confirming the reload before the negative assertion. Single-container workloads; bounded pollers that dump ContainerProfile status on timeout (never a 20m panic); Eventually-style polling on the real alert condition instead of fixed sleeps. Also fixes a latent bug: the old 'no new alert' check filtered on a process_name label that alerts never carry, so it always matched zero and passed regardless; the rewrite keys on comm (process) and rule_id (network). Verified: both pass twice on a live rig (rc5l), 0 'database is locked'. This now tests profile ENFORCEMENT (authored partial->full), not natural learning. Signed-off-by: entlein <einentlein@gmail.com>
… sleep Test_16 slept a hard-coded 175s after the restart-inducing 'service nginx stop' before exec'ing the ls violation (the WaitForContainerProfileCompletion guard was commented out), then asserted the 'Unexpected process launched' alert. On a quiet system that had ~110s of slack; under full-CT concurrency the storage single-writer SQLite contention (database is locked) defers the merged profile's completion-status write past the fixed window, so ls ran against a non-enforcing profile and no alert fired -> the assertion failed (observed as Test_16 red in a full CT with 12 lock events). Replace the fixed sleep with a bounded poll (5m, fail-fast, dumps ContainerProfiles on timeout) for the MERGED ContainerProfile reaching 'completed' after the restart -- keyed on the merged profile only (name has no -<32 hex> suffix), not WaitForContainerProfileCompletion which requires ALL matching profiles completed and would hang on the transient per-instance profiles the restart spawns. Then exec ls and poll (Eventually) for the alert. Validated 3/3 on a live rig; runtime ~241s -> ~90s since it no longer over-sleeps. Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
… us to find a way of catching it. its still not clear what caused the issue Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
committed my fork build
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
…nce when ephermal continers are not being learnt anymore Signed-off-by: entlein <einentlein@gmail.com>
… due to timeouts Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Reviewed the state of this PR before deciding on approval. This PR carries the exact same head commit ( Current state of this PR as of now:
No blockers to flag here — the code itself is under active review on #864. I'd hold off on approving this draft; once the CI run it exists to validate finishes green, that result should feed back into the #864 discussion rather than this PR being merged on its own. |
|
Ah, I need to edit the .github here to actually run the right numbers now and AI tells me this CI run used storage v0.0.298 (resolved by tests/scripts/storage-tag.sh). AI says:
|
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
|
I dont think I can push to this branch , in order to test 48 and 15 , the fastest way would be hardcoding cat /tests/scripts/storage-tag.sh Assuming this pr is ditched afterwards. Sry for the mess!!! |
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
|
@entlein done |
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
Draft PR created to trigger and validate component tests in CI (originating from #864).
Ref: #864