Skip to content

CCXDEV-15829: subscription gathering #1235

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.

Sign up for GitHub

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

Open
opokornyy wants to merge 2 commits into
base: master
Choose a base branch
from
Open

CCXDEV-15829: subscription gathering #1235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7990295
feat: add subscription gatherer
opokornyy Feb 18, 2026
1976451
feat: test subscription gatherer
opokornyy Feb 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions docs/insights-archive-sample/config/subscriptions/community-kubevirt-hyperconverged.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"apiVersion": "operators.coreos.com/v1alpha1",
"kind": "Subscription",
"metadata": {
"creationTimestamp": "2026-02-18T08:53:55Z",
"generation": 1,
"labels": {
"operators.coreos.com/community-kubevirt-hyperconverged.kubevirt-hyperconverged": ""
},
"name": "community-kubevirt-hyperconverged",
"namespace": "kubevirt-hyperconverged",
"resourceVersion": "12345",
"uid": "00000000-0000-0000-0000-000000000000"
},
"spec": {
"channel": "stable",
"installPlanApproval": "Automatic",
"name": "community-kubevirt-hyperconverged",
"source": "community-operators",
"sourceNamespace": "openshift-marketplace",
"startingCSV": "kubevirt-hyperconverged-operator.v1.16.0"
}
}
1 change: 1 addition & 0 deletions pkg/gatherers/clusterconfig/clusterconfig_gatherer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ var gatheringFunctions = map[string]gathererFuncPtr{
"silenced_alerts": (*Gatherer).GatherSilencedAlerts,
"storage_classes": (*Gatherer).GatherStorageClasses,
"storage_cluster": (*Gatherer).GatherStorageCluster,
"subscriptions": (*Gatherer).GatherSubscription,
"support_secret": (*Gatherer).GatherSupportSecret,
"tsdb_status": (*Gatherer).GatherPrometheusTSDBStatus,
"validating_webhook_configurations": (*Gatherer).GatherValidatingWebhookConfigurations,
Expand Down
75 changes: 75 additions & 0 deletions pkg/gatherers/clusterconfig/gather_subscription.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package clusterconfig

import (
"context"
"fmt"

"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/dynamic"

"github.com/openshift/insights-operator/pkg/record"
)

// GatherSubscription Collects `Subscription` from all namespaces.
//
// ### API Reference
// - https://github.com/operator-framework/api/blob/master/crds/operators.coreos.com_subscriptions.yaml
//
// ### Sample data
// - docs/insights-archive-sample/config/subscriptions/community-kubevirt-hyperconverged.json
//
// ### Location in archive
// - `config/subscriptions/{name}.json`
//
// ### Config ID
// `clusterconfig/subscriptions`
//
// ### Released version
// - 4.22
//
// ### Backported versions
// None
//
// ### Changes
// None
func (g *Gatherer) GatherSubscription(ctx context.Context) ([]record.Record, []error) {
dynamicClient, err := dynamic.NewForConfig(g.gatherKubeConfig)
if err != nil {
return nil, []error{err}
}

return gatherSubscriptions(ctx, dynamicClient)
}

func gatherSubscriptions(ctx context.Context, dynamicClient dynamic.Interface) ([]record.Record, []error) {
gvr := schema.GroupVersionResource{
Group: "operators.coreos.com",
Version: "v1alpha1",
Resource: "subscriptions",
}

// List all resources
subscriptionList, err := dynamicClient.Resource(gvr).List(ctx, metav1.ListOptions{})
if errors.IsNotFound(err) {
return nil, nil
}
if err != nil {
return nil, []error{err}
}

var records []record.Record

for i, rc := range subscriptionList.Items {
// Drop status field that is not needed
delete(subscriptionList.Items[i].Object, "status")

records = append(records, record.Record{
Name: fmt.Sprintf("config/subscriptions/%s", rc.GetName()),
Item: record.ResourceMarshaller{Resource: &subscriptionList.Items[i]},
})
Comment on lines +64 to +71
Copy link

@coderabbitai coderabbitai bot Feb 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check naming conventions for config/subscriptions and other namespaced resources
echo "=== Looking for config/subscriptions references ==="
rg -n --type=go -C2 'config/subscriptions' 

echo -e "\n=== Checking for other namespaced resource patterns ==="
rg -n --type=go 'config/[a-z]+/.*GetNamespace' 

echo -e "\n=== Looking for test files related to gather_subscription ==="
fd -e "test" | xargs grep -l "gather_subscription\|config/subscriptions" 2>/dev/null

echo -e "\n=== Checking if other resources include namespace in path ==="
rg -n --type=go 'GetNamespace.*fmt.Sprintf\|fmt.Sprintf.*GetNamespace' pkg/gatherers/clusterconfig

Repository: openshift/insights-operator

Length of output: 1839

🏁 Script executed:

#!/bin/bash
# Examine the gather_subscription.go file to understand the context better
echo "=== Full context of gather_subscription.go around lines 64-71 ==="
cat -n pkg/gatherers/clusterconfig/gather_subscription.go | sed -n '55,80p'

echo -e "\n=== Check the resource type definition ==="
rg -n "type.*Subscription" --type=go | head -20

Repository: openshift/insights-operator

Length of output: 808

🏁 Script executed:

#!/bin/bash
# Check if Subscriptions are namespace-scoped or cluster-scoped
echo "=== Looking for Subscription resource metadata/schema ==="
rg -n "Subscription" --type=go pkg/gatherers/clusterconfig/ -A5 -B5 | head -60

echo -e "\n=== Check for any test files that might validate the path format ==="
find . -name "*subscription*test*" -o -name "*test*subscription*" 2>/dev/null | head -10

Repository: openshift/insights-operator

Length of output: 5007

🏁 Script executed:

#!/bin/bash
# Check how the subscriptions are fetched - cluster or namespace scoped
echo "=== Full gatherSubscriptions function ==="
cat -n pkg/gatherers/clusterconfig/gather_subscription.go | sed -n '46,75p'

echo -e "\n=== Check dynamic client usage for list parameters ==="
rg -n "\.List\(" pkg/gatherers/clusterconfig/gather_subscription.go -A3 -B3

Repository: openshift/insights-operator

Length of output: 1390

🏁 Script executed:

#!/bin/bash
# Compare with other namespace-scoped resources to see naming patterns
echo "=== Look for other gatherers that handle namespaced resources ==="
ls -la pkg/gatherers/clusterconfig/gather_*.go | head -20

echo -e "\n=== Check for patterns in other gatherers with namespace ==="
rg -n "GetNamespace()" pkg/gatherers/clusterconfig/ --type=go | head -20

echo -e "\n=== Examine one example: check gather_pod or similar ==="
rg -n "config/.*GetNamespace\|config/.*fmt.Sprintf.*namespace" pkg/gatherers/clusterconfig/ --type=go

Repository: openshift/insights-operator

Length of output: 4789

🏁 Script executed:

#!/bin/bash
# Check what the test expects for the record path
echo "=== Check test expectations for path format ==="
cat -n pkg/gatherers/clusterconfig/gather_subscription_test.go | sed -n '80,150p'

echo -e "\n=== Look for assertions on record names ==="
rg -n "config/subscriptions\|Record{" pkg/gatherers/clusterconfig/gather_subscription_test.go -A3 -B1

Repository: openshift/insights-operator

Length of output: 2888

🏁 Script executed:

#!/bin/bash
# Check the test assertions more carefully
echo "=== Full test assertions section ==="
cat -n pkg/gatherers/clusterconfig/gather_subscription_test.go | sed -n '147,200p'

echo -e "\n=== Check if test validates record names ==="
grep -n "expectedRecordName\|assert.*Name" pkg/gatherers/clusterconfig/gather_subscription_test.go

Repository: openshift/insights-operator

Length of output: 2772

🏁 Script executed:

#!/bin/bash
# Check if the two subscriptions in the test have the same name or different names
echo "=== Check subscription names and namespaces in test YAML ==="
cat -n pkg/gatherers/clusterconfig/gather_subscription_test.go | sed -n '21,75p'

Repository: openshift/insights-operator

Length of output: 2143

Include namespace in Subscription record path to prevent collisions.

The code collects Subscriptions from all namespaces but uses only the Subscription name in the archive path. If two namespaces contain the same Subscription name, records will collide and data will be lost. This pattern is inconsistent with how other namespace-scoped resources are handled throughout the codebase (e.g., config/pdbs/{namespace}/{name}, config/logging/{namespace}/{name}, config/machines/{namespace}/{name}).

Update line 69 to include namespace in the path and add test coverage for same-named Subscriptions across different namespaces.

Proposed fix 
-			Name: fmt.Sprintf("config/subscriptions/%s", rc.GetName()),
+			Name: fmt.Sprintf("config/subscriptions/%s/%s", rc.GetNamespace(), rc.GetName()),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for i, rc := range subscriptionList.Items {
// Drop status field that is not needed
delete(subscriptionList.Items[i].Object, "status")
records = append(records, record.Record{
Name: fmt.Sprintf("config/subscriptions/%s", rc.GetName()),
Item: record.ResourceMarshaller{Resource: &subscriptionList.Items[i]},
})
for i, rc := range subscriptionList.Items {
// Drop status field that is not needed
delete(subscriptionList.Items[i].Object, "status")
records = append(records, record.Record{
Name: fmt.Sprintf("config/subscriptions/%s/%s", rc.GetNamespace(), rc.GetName()),
Item: record.ResourceMarshaller{Resource: &subscriptionList.Items[i]},
})
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/gatherers/clusterconfig/gather_subscription.go` around lines 64 - 71, The
record path for collected Subscriptions uses only rc.GetName() which can collide
across namespaces; update the record creation in the loop that iterates
subscriptionList.Items so the Name uses both namespace and name (e.g.,
fmt.Sprintf("config/subscriptions/%s/%s", rc.GetNamespace(), rc.GetName())) when
constructing the record.Record for record.ResourceMarshaller{Resource:
&subscriptionList.Items[i]}. Also add a unit/integration test that creates two
Subscription objects with the same name in different namespaces and confirms
both records are produced with distinct paths to prevent overwrite/collision.

}

return records, nil
}
Loading