Skip to content
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.

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

[vcenterreceiver] TLS settings not honored for initial GetServiceContent call #36482

Merged
merged 14 commits into from
Dec 3, 2024
Merged
27 changes: 27 additions & 0 deletions .chloggen/vcenter-tls.yaml
Original file line number Diff line number Diff line change
@@ -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]
48 changes: 27 additions & 21 deletions receiver/vcenterreceiver/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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)
Expand All @@ -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
}
Expand Down
11 changes: 3 additions & 8 deletions receiver/vcenterreceiver/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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)
})
Expand Down
11 changes: 4 additions & 7 deletions receiver/vcenterreceiver/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
Loading