diff --git a/comp/core/workloadmeta/collectors/internal/cloudfoundry/vm/cf_vm.go b/comp/core/workloadmeta/collectors/internal/cloudfoundry/vm/cf_vm.go index d4be078bed5f..ec81766744f9 100644 --- a/comp/core/workloadmeta/collectors/internal/cloudfoundry/vm/cf_vm.go +++ b/comp/core/workloadmeta/collectors/internal/cloudfoundry/vm/cf_vm.go @@ -10,6 +10,7 @@ import ( "context" "encoding/json" "fmt" + "sync" "time" "go.uber.org/fx" @@ -29,6 +30,10 @@ const ( componentName = "workloadmeta-cloudfoundry-vm" ) +// getClusterAgentClient is overridden in tests to avoid depending on the +// real cluster agent connection singleton. +var getClusterAgentClient = clusteragent.GetClusterAgentClient + type dependencies struct { fx.In @@ -45,8 +50,9 @@ type collector struct { gardenUtil cloudfoundry.GardenUtilInterface nodeName string - dcaClient clusteragent.DCAClientInterface - dcaEnabled bool + dcaClientLock sync.Mutex + dcaClient clusteragent.DCAClientInterface + dcaEnabled bool } // NewCollector instantiates a CollectorProvider which can provide a CF container collector @@ -108,7 +114,7 @@ func (c *collector) Pull(_ context.Context) error { var allContainersTags map[string][]string if dcaClient := c.getDCAClient(); dcaClient != nil { - allContainersTags, err = c.dcaClient.GetCFAppsMetadataForNode(c.nodeName) + allContainersTags, err = dcaClient.GetCFAppsMetadataForNode(c.nodeName) if err != nil { log.Debugf("Unable to fetch CF tags from cluster agent, CF tags will be missing, err: %v", err) } @@ -223,6 +229,9 @@ func (c *collector) Pull(_ context.Context) error { } func (c *collector) getDCAClient() clusteragent.DCAClientInterface { + c.dcaClientLock.Lock() + defer c.dcaClientLock.Unlock() + if !c.dcaEnabled { return nil } @@ -231,13 +240,13 @@ func (c *collector) getDCAClient() clusteragent.DCAClientInterface { return c.dcaClient } - var err error - c.dcaClient, err = clusteragent.GetClusterAgentClient() + client, err := getClusterAgentClient() if err != nil { log.Debugf("Could not initialise the communication with the cluster agent, PCF tags may be missing, err: %v", err) return nil } + c.dcaClient = client return c.dcaClient } diff --git a/comp/core/workloadmeta/collectors/internal/cloudfoundry/vm/cf_vm_test.go b/comp/core/workloadmeta/collectors/internal/cloudfoundry/vm/cf_vm_test.go index 5c76b7ab90c0..05490a883ca8 100644 --- a/comp/core/workloadmeta/collectors/internal/cloudfoundry/vm/cf_vm_test.go +++ b/comp/core/workloadmeta/collectors/internal/cloudfoundry/vm/cf_vm_test.go @@ -509,3 +509,40 @@ func TestPullAppNameWithGardenPropertiesWithoutDCA(t *testing.T) { assert.Contains(t, container.CollectorTags, "container_name:"+"app-name-1") } + +// TestPullDCAUnreachableTwice reproduces the typed-nil interface trap: when +// clusteragent.GetClusterAgentClient returns a concrete (*DCAClient)(nil) +// alongside an error, assigning it into an interface field before the error +// check poisons the field with a non-nil interface wrapping a nil pointer. +// A second Pull would then see c.dcaClient != nil, skip re-init, and call a +// method on a nil *DCAClient receiver. +func TestPullDCAUnreachableTwice(t *testing.T) { + fakeGardenUtil := FakeGardenUtil{containers: nil} + + orig := getClusterAgentClient + getClusterAgentClient = func() (*clusteragent.DCAClient, error) { + // Simulate GetClusterAgentClient's real error path: a nil concrete + // pointer alongside a non-nil error. + return nil, fmt.Errorf("cluster agent unreachable") + } + t.Cleanup(func() { getClusterAgentClient = orig }) + + workloadmetaStore := fxutil.Test[workloadmetamock.Mock](t, fx.Options( + core.MockBundle(), + workloadmetafxmock.MockModule(workloadmeta.NewParams()), + )) + + c := collector{ + gardenUtil: &fakeGardenUtil, + store: workloadmetaStore, + dcaEnabled: true, + } + + require.NoError(t, c.Pull(context.TODO())) + require.Nil(t, c.dcaClient, "first Pull must not poison the dcaClient field on error") + + require.NotPanics(t, func() { + require.NoError(t, c.Pull(context.TODO())) + }) + require.Nil(t, c.dcaClient, "second Pull must not poison the dcaClient field on error") +}