Skip to content

Commit

Permalink
Bypass lookup svc for STS and SSOadmin clients
Browse files Browse the repository at this point in the history
- For local clients on vCenter, lookup service can be omitted and local
  Envoy processes can be used instead, along with 'system-' prefixed
  variants of SSOAdmin/STS. This avoids the need to discover the
  vCenter SSO domain and avoids an extra round trip.

- Also extends simulator to allow aliasing the same SDK to multiple
  paths. Moves vimService, ssoadmin, STS to this model.

- Using env vars to allow testing this with the simulator in unit tests,
  but with a sensible defaults (localhost/1080).

- Added unit tests for this path (simulate lookupsvc returning the wrong
  URL and asserting that the STS/ssoadmin clients still function)

- Moved the lookup service simulator breaking logic to
  lookup/simulator.go so it's usable elsewhere.

- Tweaked the simulator.Test/Run() methods to not always set up TLS
  config (since we use HTTP for the ssoadmin/sts client)

- Moved sts/client_test.go to the sts_test package to avoid an import
  cycle (sts/simulator -> sts -> sts/simulator via test code)
  • Loading branch information
mayankbh committed Jul 26, 2023
1 parent 1a627f9 commit c6a9ac8
Show file tree
Hide file tree
Showing 12 changed files with 238 additions and 102 deletions.
19 changes: 18 additions & 1 deletion internal/helpers.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (c) 2020 VMware, Inc. All Rights Reserved.
Copyright (c) 2020-2023 VMware, Inc. All Rights Reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -18,8 +18,10 @@ package internal

import (
"net"
"os"
"path"

"github.com/vmware/govmomi/vim25"
"github.com/vmware/govmomi/vim25/mo"
"github.com/vmware/govmomi/vim25/types"
)
Expand Down Expand Up @@ -61,3 +63,18 @@ func HostSystemManagementIPs(config []types.VirtualNicManagerNetConfig) []net.IP

return ips
}

// UsingEnvoySidecar determines if the given *vim25.Client is using vCenter's
// local Envoy sidecar (as opposed to using the HTTPS port.)
// Returns a boolean indicating whether to use the sidecar or not.
func UsingEnvoySidecar(c *vim25.Client) bool {
envoySidecarPort := os.Getenv("GOVMOMI_ENVOY_SIDECAR_PORT")
if envoySidecarPort == "" {
envoySidecarPort = "1080"
}
envoySidecarHost := os.Getenv("GOVMOMI_ENVOY_SIDECAR_HOST")
if envoySidecarHost == "" {
envoySidecarHost = "localhost"
}
return c.URL().Hostname() == envoySidecarHost && c.URL().Scheme == "http" && c.URL().Port() == envoySidecarPort
}
27 changes: 27 additions & 0 deletions internal/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,16 @@ limitations under the License.
package internal_test

import (
"fmt"
"net/url"
"testing"

"github.com/stretchr/testify/require"

"github.com/vmware/govmomi/internal"
"github.com/vmware/govmomi/simulator/esx"
"github.com/vmware/govmomi/vim25"
"github.com/vmware/govmomi/vim25/soap"
)

func TestHostSystemManagementIPs(t *testing.T) {
Expand All @@ -30,3 +36,24 @@ func TestHostSystemManagementIPs(t *testing.T) {
t.Fatalf("Expected management ip %s, got %s", "127.0.0.1", ips[0].String())
}
}

func TestUsingVCEnvoySidecar(t *testing.T) {
t.Run("VC HTTPS port", func(t *testing.T) {
scheme := "https"
hostname := "my-vcenter"
port := 443
u := &url.URL{Scheme: scheme, Host: fmt.Sprintf("%s:%d", hostname, port)}
client := &vim25.Client{Client: soap.NewClient(u, true)}
usingSidecar := internal.UsingEnvoySidecar(client)
require.False(t, usingSidecar)
})
t.Run("Envoy sidecar", func(t *testing.T) {
scheme := "http"
hostname := "localhost"
port := 1080
u := &url.URL{Scheme: scheme, Host: fmt.Sprintf("%s:%d", hostname, port)}
client := &vim25.Client{Client: soap.NewClient(u, true)}
usingSidecar := internal.UsingEnvoySidecar(client)
require.True(t, usingSidecar)
})
}
19 changes: 2 additions & 17 deletions lookup/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,31 +30,16 @@ import (
"github.com/vmware/govmomi/sts"
"github.com/vmware/govmomi/vim25"

_ "github.com/vmware/govmomi/lookup/simulator"
lsim "github.com/vmware/govmomi/lookup/simulator"
_ "github.com/vmware/govmomi/ssoadmin/simulator"
_ "github.com/vmware/govmomi/sts/simulator"
)

// make the path of all lookup service urls invalid
func breakLookupServiceURLs() {
setting := simulator.Map.OptionManager().Setting

for _, s := range setting {
o := s.GetOptionValue()
if strings.HasSuffix(o.Key, ".uri") {
val := o.Value.(string)
u, _ := url.Parse(val)
u.Path = "/enoent" + u.Path
o.Value = u.String()
}
}
}

// test lookup.EndpointURL usage by the ssoadmin and sts clients
func TestEndpointURL(t *testing.T) {
// these client calls should fail since we'll break the URL paths
simulator.Test(func(ctx context.Context, vc *vim25.Client) {
breakLookupServiceURLs()
lsim.BreakLookupServiceURLs()

{
_, err := ssoadmin.NewClient(ctx, vc)
Expand Down
17 changes: 17 additions & 0 deletions lookup/simulator/simulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package simulator

import (
"net/url"
"strings"
"sync"

"github.com/vmware/govmomi/lookup"
Expand Down Expand Up @@ -175,3 +177,18 @@ func (s *ServiceRegistration) List(req *types.List) soap.HasFault {

return body
}

// BreakLookupServiceURLs makes the path of all lookup service urls invalid
func BreakLookupServiceURLs() {
setting := simulator.Map.OptionManager().Setting

for _, s := range setting {
o := s.GetOptionValue()
if strings.HasSuffix(o.Key, ".uri") {
val := o.Value.(string)
u, _ := url.Parse(val)
u.Path = "/enoent" + u.Path
o.Value = u.String()
}
}
}
3 changes: 2 additions & 1 deletion simulator/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -876,9 +876,10 @@ func (m *Model) Run(f func(context.Context, *vim25.Client) error) error {
if err != nil {
return err
}
// Only force TLS if the provided model didn't have any Service.
m.Service.TLS = new(tls.Config)
}

m.Service.TLS = new(tls.Config)
m.Service.RegisterEndpoints = true

s := m.Service.NewServer()
Expand Down
16 changes: 10 additions & 6 deletions simulator/simulator.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (c) 2017-2018 VMware, Inc. All Rights Reserved.
Copyright (c) 2017-2023 VMware, Inc. All Rights Reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -421,7 +421,9 @@ func (s *Service) HandleFunc(pattern string, handler func(http.ResponseWriter, *

// RegisterSDK adds an HTTP handler for the Registry's Path and Namespace.
// If r.Path is already registered, r's objects are added to the existing Registry.
func (s *Service) RegisterSDK(r *Registry) {
// An optional set of aliases can be provided to register the same handler for
// multiple paths.
func (s *Service) RegisterSDK(r *Registry, alias ...string) {
if existing, ok := s.sdk[r.Path]; ok {
for id, obj := range r.objects {
existing.objects[id] = obj
Expand All @@ -435,6 +437,11 @@ func (s *Service) RegisterSDK(r *Registry) {

s.sdk[r.Path] = r
s.ServeMux.HandleFunc(r.Path, s.ServeSDK)

for _, p := range alias {
s.sdk[p] = r
s.ServeMux.HandleFunc(p, s.ServeSDK)
}
}

// StatusSDK can be used to simulate an /sdk HTTP response code other than 200.
Expand Down Expand Up @@ -654,12 +661,9 @@ func defaultIP(addr *net.TCPAddr) string {

// NewServer returns an http Server instance for the given service
func (s *Service) NewServer() *Server {
s.RegisterSDK(Map)
s.RegisterSDK(Map, Map.Path+"/vimService")

mux := s.ServeMux
vim := Map.Path + "/vimService"
s.sdk[vim] = s.sdk[vim25.Path]
mux.HandleFunc(vim, s.ServeSDK)
mux.HandleFunc(Map.Path+"/vimServiceVersions.xml", s.ServiceVersions)
mux.HandleFunc(folderPrefix, s.ServeDatastore)
mux.HandleFunc(guestPrefix, ServeGuest)
Expand Down
33 changes: 25 additions & 8 deletions ssoadmin/client.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (c) 2018-2022 VMware, Inc. All Rights Reserved.
Copyright (c) 2018-2023 VMware, Inc. All Rights Reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -18,11 +18,13 @@ package ssoadmin

import (
"context"
"fmt"
"math"
"path"
"reflect"
"strings"

"github.com/vmware/govmomi/internal"
"github.com/vmware/govmomi/lookup"
ltypes "github.com/vmware/govmomi/lookup/types"
"github.com/vmware/govmomi/ssoadmin/methods"
Expand All @@ -33,9 +35,11 @@ import (
)

const (
Namespace = "sso"
Version = "version2"
Path = "/sso-adminserver" + vim25.Path
Namespace = "sso"
Version = "version2"
basePath = "/sso-adminserver"
Path = basePath + vim25.Path
SystemPath = basePath + "/system-sdk"
)

var (
Expand Down Expand Up @@ -63,7 +67,16 @@ func init() {
vim.Add("SsoAdminFaultDuplicateSolutionCertificateFaultFault", reflect.TypeOf((*vim.InvalidArgument)(nil)).Elem())
}

func NewClient(ctx context.Context, c *vim25.Client) (*Client, error) {
func getEndpointURL(ctx context.Context, c *vim25.Client) string {
// Services running on vCenter can bypass lookup service using the
// system-sdk path. This avoids the need to lookup the system domain.
if useSidecar := internal.UsingEnvoySidecar(c); useSidecar {
return fmt.Sprintf("http://%s%s", c.URL().Host, SystemPath)
}
return getEndpointURLFromLookupService(ctx, c)
}

func getEndpointURLFromLookupService(ctx context.Context, c *vim25.Client) string {
filter := &ltypes.LookupServiceRegistrationFilter{
ServiceType: &ltypes.LookupServiceRegistrationServiceType{
Product: "com.vmware.cis",
Expand All @@ -75,8 +88,12 @@ func NewClient(ctx context.Context, c *vim25.Client) (*Client, error) {
},
}

url := lookup.EndpointURL(ctx, c, Path, filter)
sc := c.Client.NewServiceClient(url, Namespace)
return lookup.EndpointURL(ctx, c, Path, filter)
}

func NewClient(ctx context.Context, c *vim25.Client) (*Client, error) {
url := getEndpointURL(ctx, c)
sc := c.NewServiceClient(url, Namespace)
sc.Version = Version

admin := &Client{
Expand All @@ -85,7 +102,7 @@ func NewClient(ctx context.Context, c *vim25.Client) (*Client, error) {
Domain: "vsphere.local", // Default
Limit: math.MaxInt32,
}
if url != Path {
if url != Path && !internal.UsingEnvoySidecar(c) {
admin.Domain = path.Base(url)
}

Expand Down
78 changes: 44 additions & 34 deletions ssoadmin/client_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (c) 2018 VMware, Inc. All Rights Reserved.
Copyright (c) 2018-2023 VMware, Inc. All Rights Reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -14,47 +14,57 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package ssoadmin
package ssoadmin_test

import (
"context"
"log"
"os"
"testing"

"github.com/stretchr/testify/require"

lsim "github.com/vmware/govmomi/lookup/simulator"
"github.com/vmware/govmomi/simulator"
"github.com/vmware/govmomi/ssoadmin"
_ "github.com/vmware/govmomi/ssoadmin/simulator"
"github.com/vmware/govmomi/ssoadmin/types"
_ "github.com/vmware/govmomi/sts/simulator"
"github.com/vmware/govmomi/vim25"
"github.com/vmware/govmomi/vim25/soap"
)

func TestClient(t *testing.T) {
ctx := context.Background()
url := os.Getenv("GOVC_TEST_URL")
if url == "" {
t.SkipNow()
}

u, err := soap.ParseURL(url)
if err != nil {
t.Fatal(err)
}

c, err := vim25.NewClient(ctx, soap.NewClient(u, true))
if err != nil {
log.Fatal(err)
}

if !c.IsVC() {
t.SkipNow()
}

admin, err := NewClient(ctx, c)
if err != nil {
t.Fatal(err)
}

if err = admin.Login(ctx); err == nil {
t.Error("expected error") // soap.Header.Security not set
}

// sts/client_test.go tests the success paths
t.Run("Happy path using lookup service", func(t *testing.T) {
simulator.Test(func(ctx context.Context, client *vim25.Client) {
c, err := ssoadmin.NewClient(ctx, client)
require.NoError(t, err)

verifyClient(t, ctx, c)
})
})
t.Run("With Envoy sidecar and a malfunctioning lookup service, ssoadmin client creation should still succeed", func(t *testing.T) {
model := simulator.VPX()
model.Create()
simulator.Test(func(ctx context.Context, client *vim25.Client) {
// Map Envoy sidecar on the same port as the vcsim client.
os.Setenv("GOVMOMI_ENVOY_SIDECAR_PORT", client.Client.URL().Port())
os.Setenv("GOVMOMI_ENVOY_SIDECAR_HOST", client.Client.URL().Hostname())

lsim.BreakLookupServiceURLs()

c, err := ssoadmin.NewClient(ctx, client)
require.NoError(t, err)

verifyClient(t, ctx, c)
}, model)
})
}

func verifyClient(t *testing.T, ctx context.Context, c *ssoadmin.Client) {
err := c.CreatePersonUser(ctx, "testuser", types.AdminPersonDetails{FirstName: "test", LastName: "user"}, "password")
require.NoError(t, err)

user, err := c.FindUser(ctx, "testuser")
require.NoError(t, err)
require.Equal(t, &types.AdminUser{Id: types.PrincipalId{Name: "testuser", Domain: "vsphere.local"}, Kind: "person"}, user)

}
2 changes: 1 addition & 1 deletion ssoadmin/simulator/simulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
func init() {
simulator.RegisterEndpoint(func(s *simulator.Service, r *simulator.Registry) {
if r.IsVPX() {
s.RegisterSDK(New(r, s.Listen))
s.RegisterSDK(New(r, s.Listen), ssoadmin.SystemPath)
}
})
}
Expand Down
Loading

0 comments on commit c6a9ac8

Please sign in to comment.