operator-utils icon indicating copy to clipboard operation
operator-utils copied to clipboard

Fix issue #194: Remove fields with value 0 when conditionals change

Open ephico2real2 opened this issue 2 months ago • 0 comments

PR-194: Remove fields present in actual but missing in expected (handles zero-value cases like "0")

Summary

This change fixes a bug where fields with "zero-like" values (e.g., "0") were not removed when conditionals stop rendering them in templates. The comparison/patch logic previously didn’t emit deletions for keys missing in expected but present in actual.

Implementation

  • operator-utils fork/branch/commit:
    • https://github.com/ephico2real2/operator-utils/tree/fix-issue-194-field-removal-zero-value
    • Commit: 9569465
    • New logic: createPatchWithNullFields + addNullFieldsForMissing inject null into JSON merge patches for keys present only in actual, so Kubernetes removes them.

How reviewers can reproduce and verify (using my operator fork)

  • Test harness repo (namespace-configuration-operator):
    • https://github.com/ephico2real2/namespace-configuration-operator/tree/feature/finalizer-fixes-template-filtering-tests
    • Working branch: feature/finalizer-fixes-template-filtering-tests
  • Consolidated docs in that repo/branch:
    • examples/test-and-logic/ISSUE-194-ROOT-CAUSE-SUMMARY.md
    • examples/test-and-logic/ISSUE-194-FIX-IMPLEMENTATION.md
    • examples/test-and-logic/ISSUE-194-VERIFICATION-GUIDE.md

Wire the fixed dependency

Option A (track branch):

# In namespace-configuration-operator
go get github.com/ephico2real2/operator-utils@fix-issue-194-field-removal-zero-value
go mod tidy

Option B (pin exact commit via pseudo-version):

check examples/test-and-logic/ISSUE-194-FIX-IMPLEMENTATION.md for directions.


// go.mod
replace github.com/redhat-cop/operator-utils => github.com/ephico2real2/operator-utils v0.0.0-20251208075852-9569465257c1

Build & run locally (in namespace-configuration-operator)

./build.sh -o bin/manager main.go
./run-go.sh --skip-build

Test config

examples/test-and-logic/test-issue-194-field-removal-namespaceconfig.yaml
- Matches namespaces labeled test-issue-194=true
- Conditional: if allow-pvc != "true", include spec.hard.persistentvolumeclaims: "0"

Verification steps

  1. Initial (no annotation) → field present:
oс create namespace test-issue-194-ns || true
oc label namespace test-issue-194-ns test-issue-194=true --overwrite
oc apply -f examples/test-and-logic/test-issue-194-field-removal-namespaceconfig.yaml
oc get resourcequota test-issue-194-quota -n test-issue-194-ns -o jsonpath='{.spec.hard.persistentvolumeclaims}' && echo
# Expect: 0
  1. Set annotation allow-pvc=true → field removed:
oc annotate namespace test-issue-194-ns allow-pvc=true --overwrite
sleep 8
oc get resourcequota test-issue-194-quota -n test-issue-194-ns -o jsonpath='{.spec.hard.persistentvolumeclaims}' && echo
# Expect: (empty)
  1. Remove annotation → field added back:
oc annotate namespace test-issue-194-ns allow-pvc-
sleep 8
oc get resourcequota test-issue-194-quota -n test-issue-194-ns -o jsonpath='{.spec.hard.persistentvolumeclaims}' && echo
# Expect: 0

Real-time proof (timestamps + full YAML)

Server-side apply captures managedFields.time:

oc apply -f examples/test-and-logic/test-issue-194-field-removal-namespaceconfig.yaml --server-side --field-manager=issue-194-test
oc get namespaceconfig test-issue-194-field-removal -o json | jq -r '.metadata.managedFields | sort_by(.time) | last | .time'

Capture and diff YAML:

oc get resourcequota test-issue-194-quota -n test-issue-194-ns -o yaml > /tmp/rq-before.yaml
oc annotate namespace test-issue-194-ns allow-pvc=true --overwrite && sleep 8
oc get resourcequota test-issue-194-quota -n test-issue-194-ns -o yaml > /tmp/rq-after.yaml
diff -u /tmp/rq-before.yaml /tmp/rq-after.yaml | sed -n '1,200p'

Live YAML excerpt (after fix — allow-pvc=true) and the original template snippet are embedded in:

  • examples/test-and-logic/ISSUE-194-VERIFICATION-GUIDE.md

Commands used to identify the correct module

grep -r "func.*UpdateLockedResources" controllers/

go doc github.com/redhat-cop/operator-utils/pkg/util/lockedresourcecontroller.EnforcingReconciler.UpdateLockedResources

grep -A 5 "type NamespaceConfigReconciler struct" controllers/namespaceconfig_controller.go

grep -B 2 -A 2 "UpdateLockedResources" controllers/namespaceconfig_controller.go

go list -m -versions github.com/redhat-cop/operator-utils

go list -m github.com/redhat-cop/operator-utils

Impact & compatibility

  • Generic fix for any resource/field removed by conditional rendering.
  • Uses standard JSON Merge Patch semantics (null deletes); respects excluded paths; supports nested structures.

Checklist

  • [x] Fix implemented with recursive null-injection for missing keys

  • [x] Verified locally and on a cluster with before/after YAML diffs

  • [x] No changes to public API of operator-utils

  • [x] Backwards compatible

  • Add createPatchWithNullFields to set missing fields to null in merge patches

  • Add addNullFieldsForMissing helper to recursively handle nested structures

  • Ensures fields are properly removed when they should be absent

  • Fixes bug where fields with value "0" are not removed when conditionals change from true to false

ephico2real2 avatar Dec 08 '25 09:12 ephico2real2