From b3f459b5961b49df2b68f961707048dd9d3d820a Mon Sep 17 00:00:00 2001 From: Sam DeHaan Date: Tue, 3 Dec 2024 14:56:44 -0500 Subject: [PATCH] [vcenterreceiver] TLS settings not honored for initial GetServiceContent call (#36482) #### Description The `govmomi` client used in the receiver attempts to validate the connection to vcenter before the existing code sets the TLS options (other than insecure) in the client. This is a limitation of the `govmomi` wrapper, as discussed on this issue: https://github.com/vmware/govmomi/issues/1200 . #### Link to tracking issue Related issue in Grafana Alloy: https://github.com/grafana/alloy/issues/193 #### Testing ~~This has not been tested, I would appreciate the assistance of any codeowner that could test.~~ See comments on the PR for test. --------- Co-authored-by: Daniel Jaglowski --- .chloggen/vcenter-tls.yaml | 27 +++++++++++ receiver/vcenterreceiver/client.go | 48 +++++++++++--------- receiver/vcenterreceiver/client_test.go | 11 ++--- receiver/vcenterreceiver/integration_test.go | 11 ++--- 4 files changed, 61 insertions(+), 36 deletions(-) create mode 100644 .chloggen/vcenter-tls.yaml diff --git a/.chloggen/vcenter-tls.yaml b/.chloggen/vcenter-tls.yaml new file mode 100644 index 000000000000..447ffc26f4ca --- /dev/null +++ b/.chloggen/vcenter-tls.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: vcenterreceiver + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: The existing code did not honor TLS settings beyond 'insecure'. All TLS client config should now be honored. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [36482] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user] diff --git a/receiver/vcenterreceiver/client.go b/receiver/vcenterreceiver/client.go index 094f8c509115..f72c59b07815 100644 --- a/receiver/vcenterreceiver/client.go +++ b/receiver/vcenterreceiver/client.go @@ -14,10 +14,10 @@ import ( "strings" "time" - "github.com/vmware/govmomi" "github.com/vmware/govmomi/find" "github.com/vmware/govmomi/object" "github.com/vmware/govmomi/performance" + "github.com/vmware/govmomi/session" "github.com/vmware/govmomi/view" "github.com/vmware/govmomi/vim25" "github.com/vmware/govmomi/vim25/mo" @@ -30,14 +30,14 @@ import ( // vcenterClient is a client that collects data from a vCenter endpoint. type vcenterClient struct { - logger *zap.Logger - moClient *govmomi.Client - vimDriver *vim25.Client - vsanDriver *vsan.Client - finder *find.Finder - pm *performance.Manager - vm *view.Manager - cfg *Config + logger *zap.Logger + sessionManager *session.Manager + vimDriver *vim25.Client + vsanDriver *vsan.Client + finder *find.Finder + pm *performance.Manager + vm *view.Manager + cfg *Config } var newVcenterClient = defaultNewVcenterClient @@ -51,8 +51,8 @@ func defaultNewVcenterClient(l *zap.Logger, c *Config) *vcenterClient { // EnsureConnection will establish a connection to the vSphere SDK if not already established func (vc *vcenterClient) EnsureConnection(ctx context.Context) error { - if vc.moClient != nil { - sessionActive, _ := vc.moClient.SessionManager.SessionIsActive(ctx) + if vc.sessionManager != nil { + sessionActive, _ := vc.sessionManager.SessionIsActive(ctx) if sessionActive { return nil } @@ -62,24 +62,30 @@ func (vc *vcenterClient) EnsureConnection(ctx context.Context) error { if err != nil { return err } - client, err := govmomi.NewClient(ctx, sdkURL, vc.cfg.Insecure) - if err != nil { - return fmt.Errorf("unable to connect to vSphere SDK on listed endpoint: %w", err) - } + + soapClient := soap.NewClient(sdkURL, vc.cfg.Insecure) tlsCfg, err := vc.cfg.LoadTLSConfig(ctx) if err != nil { return err } if tlsCfg != nil { - client.DefaultTransport().TLSClientConfig = tlsCfg + soapClient.DefaultTransport().TLSClientConfig = tlsCfg + } + + client, err := vim25.NewClient(ctx, soapClient) + if err != nil { + return fmt.Errorf("unable to connect to vSphere SDK on listed endpoint: %w", err) } + + sessionManager := session.NewManager(client) + user := url.UserPassword(vc.cfg.Username, string(vc.cfg.Password)) - err = client.Login(ctx, user) + err = sessionManager.Login(ctx, user) if err != nil { return fmt.Errorf("unable to login to vcenter sdk: %w", err) } - vc.moClient = client - vc.vimDriver = client.Client + vc.sessionManager = sessionManager + vc.vimDriver = client vc.finder = find.NewFinder(vc.vimDriver) vc.pm = performance.NewManager(vc.vimDriver) vc.vm = view.NewManager(vc.vimDriver) @@ -94,8 +100,8 @@ func (vc *vcenterClient) EnsureConnection(ctx context.Context) error { // Disconnect will logout of the autenticated session func (vc *vcenterClient) Disconnect(ctx context.Context) error { - if vc.moClient != nil { - return vc.moClient.Logout(ctx) + if vc.sessionManager != nil { + return vc.sessionManager.Logout(ctx) } return nil } diff --git a/receiver/vcenterreceiver/client_test.go b/receiver/vcenterreceiver/client_test.go index 49576b5df859..8d5edbdde5a1 100644 --- a/receiver/vcenterreceiver/client_test.go +++ b/receiver/vcenterreceiver/client_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/stretchr/testify/require" - "github.com/vmware/govmomi" "github.com/vmware/govmomi/find" "github.com/vmware/govmomi/performance" "github.com/vmware/govmomi/session" @@ -292,10 +291,6 @@ func TestEmptyVAppInventoryListObjects(t *testing.T) { func TestSessionReestablish(t *testing.T) { simulator.Test(func(ctx context.Context, c *vim25.Client) { sm := session.NewManager(c) - moClient := &govmomi.Client{ - Client: c, - SessionManager: sm, - } pw, _ := simulator.DefaultLogin.Password() client := vcenterClient{ vimDriver: c, @@ -307,19 +302,19 @@ func TestSessionReestablish(t *testing.T) { Insecure: true, }, }, - moClient: moClient, + sessionManager: sm, } err := sm.Logout(ctx) require.NoError(t, err) - connected, err := client.moClient.SessionManager.SessionIsActive(ctx) + connected, err := client.sessionManager.SessionIsActive(ctx) require.NoError(t, err) require.False(t, connected) err = client.EnsureConnection(ctx) require.NoError(t, err) - connected, err = client.moClient.SessionManager.SessionIsActive(ctx) + connected, err = client.sessionManager.SessionIsActive(ctx) require.NoError(t, err) require.True(t, connected) }) diff --git a/receiver/vcenterreceiver/integration_test.go b/receiver/vcenterreceiver/integration_test.go index 13d2e9f3c74c..b8c1fca70fc5 100644 --- a/receiver/vcenterreceiver/integration_test.go +++ b/receiver/vcenterreceiver/integration_test.go @@ -12,7 +12,6 @@ import ( "time" "github.com/stretchr/testify/require" - "github.com/vmware/govmomi" "github.com/vmware/govmomi/find" "github.com/vmware/govmomi/session" "github.com/vmware/govmomi/simulator" @@ -80,12 +79,10 @@ func TestIntegration(t *testing.T) { s := session.NewManager(c) newVcenterClient = func(l *zap.Logger, cfg *Config) *vcenterClient { client := &vcenterClient{ - logger: l, - cfg: cfg, - moClient: &govmomi.Client{ - Client: c, - SessionManager: s, - }, + logger: l, + cfg: cfg, + sessionManager: s, + vimDriver: c, } require.NoError(t, client.EnsureConnection(context.Background())) client.vimDriver = c