THREESCALE-16302 Remove cmp.Diff of openapi structs

[borisurbanik]

Fixes:

• https://redhat.atlassian.net/browse/THREESCALE-16302

Validation

Set up minimal 3scale dev environment.

oc new-project $NAMESPACE
make cluster/create/system-mysql
make cluster/create/system-redis
make cluster/create/backend-redis

cat << EOF | oc create -f -
kind: Secret
apiVersion: v1
metadata:
  name: s3-credentials
  namespace: $NAMESPACE
data:
  AWS_ACCESS_KEY_ID: c29tZXRoaW5nCg==
  AWS_BUCKET: c29tZXRoaW5nCg==
  AWS_REGION: dXMtd2VzdC0xCg==
  AWS_SECRET_ACCESS_KEY: c29tZXRoaW5nCg==
type: Opaque
EOF

DOMAIN=$(oc get routes console -n openshift-console -o json | jq -r '.status.ingress[0].routerCanonicalHostname' | sed 's/router-default.//')
cat << EOF | oc create -f -
kind: APIManager
apiVersion: apps.3scale.net/v1alpha1
metadata:
  name: 3scale
  namespace: $NAMESPACE
spec:
  wildcardDomain: $DOMAIN
  system:
    fileStorage:
      simpleStorageService:
        configurationSecretRef:
          name: s3-credentials
  backend:
  externalComponents:
    backend:
      redis: true
    system:
      database: true
      redis: true
EOF

Run the operator locally.

make run

Wait for reconciliation. Then test the OpenApi will be created with schema with references:

cat << 'EOF' > test-activedoc.yaml
apiVersion: capabilities.3scale.net/v1beta1
kind: ActiveDoc
metadata:
  name: test-activedoc
  annotations:
    insecure_skip_verify: "true"
spec:
  name: "Test ActiveDoc"
  systemName: "test-activedoc"
  activeDocOpenAPIRef:
    url: "https://gist.githubusercontent.com/borisurbanik/5077eec278186852e2ccc1e6d790a6cf/raw/b6bbfccf6c21938ce1d7a2bd5c16fc057d44b5e2/openapi.json"
EOF
kubectl apply -f test-activedoc.yaml

Expected to not show error:

oc get activedoc test-activedoc -o json | jq -r .status
{
  "activeDocId": 3,
  "conditions": [
    {
      "lastTransitionTime": "2026-09-10T19:47:36Z",
      "status": "False",
      "type": "Failed"
    },
    {
      "lastTransitionTime": "2026-09-10T19:47:36Z",
      "status": "False",
      "type": "Invalid"
    },
    {
      "lastTransitionTime": "2026-09-10T19:47:36Z",
      "status": "False",
      "type": "Orphan"
    },
    {
      "lastTransitionTime": "2026-09-10T19:47:36Z",
      "status": "True",
      "type": "Ready"
    }
  ],
  "observedGeneration": 1,
  "providerAccountHost": "https://3scale-admin.apps.burbanik-3scale2.cp.fyre.ibm.com"
}

And the operator (make run) to not crash.

[briangallagher]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 44.67%. Comparing base (c59a4c8) to head (52a3b90).
⚠️ Report is 18 commits behind head on master.

Files with missing lines	Patch %	Lines
...rs/capabilities/activedoc_threescale_reconciler.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1192      +/-   ##
==========================================
+ Coverage   44.03%   44.67%   +0.63%     
==========================================
  Files         204      208       +4     
  Lines       20960    21227     +267     
==========================================
+ Hits         9230     9483     +253     
- Misses      10933    10944      +11     
- Partials      797      800       +3     

Flag	Coverage Δ
unit	44.67% <0.00%> (+0.63%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
apis/apps/v1alpha1 (u)	63.56% <ø> (ø)
apis/capabilities/v1alpha1 (u)	3.50% <ø> (ø)
apis/capabilities/v1beta1 (u)	20.21% <ø> (ø)
controllers (i)	12.61% <82.35%> (+0.52%)	⬆️
pkg (u)	64.26% <92.77%> (+0.56%)	⬆️

Files with missing lines	Coverage Δ
...rs/capabilities/activedoc_threescale_reconciler.go	0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tkan145]

There are other cmp.Diff in the code, can we remove them all? Keep the log though, I don't see why we need to call cmp.Diff to compare string/bool/int.

[borisurbanik]

There are other cmp.Diff in the code, can we remove them all? Keep the log though, I don't see why we need to call cmp.Diff to compare string/bool/int.

Removed all cmp.Diff occurrences.

[tkan145]

Can we keep those log values please 😅

[borisurbanik]

Can we keep those log values please 😅

Updated, added "existing" and "desired" kv entries for the logger. Please take a look again.

[tkan145]

Most of the fields are pointers, so we are just recording their memory addresses. To record the actual values, we need to dereference those pointers; at the same time, we must ensure they are not nil. The log level is currently set to debug, and while I still find them useful for troubleshooting, it seems we need extra logic to ensure the panic error doesn't recur.

So what do you think about removing these debug logs?

[borisurbanik]

Most of the fields are pointers, so we are just recording their memory addresses. To record the actual values, we need to dereference those pointers; at the same time, we must ensure they are not nil. The log level is currently set to debug, and while I still find them useful for troubleshooting, it seems we need extra logic to ensure the panic error doesn't recur.

So what do you think about removing these debug logs?

To address directly the dereferencing while we're discussing I have added ptr.Deref - this should be very safe since it uses generics to enforce the type is known at compile time.

If it still feels like unnecessary risk I'd suggest removing the values but leaving the bare log messages to at least guide to the right field in the payload to investigate.

So, there are still the 2 other options:

• Revert the code to use cmp.Diff: Searching for it in the codebase shows other places where the cmp.Diff pattern to log differences for simple types (such as *string) is used. I think reverting would reduce LOC modified in the PR to almost nothing and be almost as safe as ptr.Deref (just without the generic type check for the future).
• Remove the debug logs entirely, or remove the values and leave just messages: looks like the default deployment at least in the upstream bundle is using info level anyway.

Let me know how does ptr.Deref look to you, or if one of the other options would be better.

[borisurbanik]

/retest

[borisurbanik]

@tkan145 Since the PR is about the bug THREESCALE-16302 and not the field diff logging, can we pick one of the 2 paths forward:

1. Revert scalar pointers to cmp.Diff: cmp.Diff already handles pointer formatting and dereferencing. The bug in THREESCALE-16302 was strictly on the cyclic openapi3.T body field - so the PR will end up only modifying that single field section.
2. Remove the per-field debug logs entirely: Remove all V(1) field update logs and keep only s.logger.V(1).Info("Desired ActiveDoc needs sync").

Which of these two would you prefer to approve?

[tkan145]

Sorry, but I find this a bit confusing. I previously asked if you wanted to remove that log section, and then you suggested using ptr and submitted a patch; I already left a comment, but are we back to square one now? Perhaps I should ask what do you want to do now?

[borisurbanik]

Sorry, but I find this a bit confusing. I previously asked if you wanted to remove that log section, and then you suggested using ptr and submitted a patch; I already left a comment, but are we back to square one now? Perhaps I should ask what do you want to do now?

I have removed the individual field debug logs as you suggested. Please re-review.

p.s.: sorry for confusion, it wasn't intentional - I think consensus has been reached now