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

fix: lookup client to use envoy sidecar url if enabled #3383

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

ramineni
Copy link
Contributor

@ramineni ramineni commented Mar 8, 2024

currently, lookup client by default fetches the uri from "config.vpxd.sso.sts.uri" value if service content set . With envoy sidecar enabled, this MR is to first check for envoy sidecar url if present and use the same.

@ramineni
Copy link
Contributor Author

ramineni commented Mar 8, 2024

cc @mayankbh

Copy link
Member

@dougm dougm left a comment

Choose a reason for hiding this comment

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

Thanks @ramineni . Can you fixup the commit message to include a fix: prefix? This is for the ChangeLog generator, see: https://github.com/vmware/govmomi/blob/main/CONTRIBUTING.md#format-of-the-commit-message

The Scheme and Host will default to those values when empty, see:

if u.Host == "" {
u.Scheme = vc.Scheme
u.Host = vc.Host
}

So just the following (untested) change below should work:

@@ -57,7 +58,7 @@ type Client struct {
 func NewClient(ctx context.Context, c *vim25.Client) (*Client, error) {
 	// PSC may be external, attempt to derive from sts.uri
 	path := &url.URL{Path: Path}
-	if c.ServiceContent.Setting != nil {
+	if !internal.UsingEnvoySidecar(c) && c.ServiceContent.Setting != nil {
 		m := object.NewOptionManager(c, *c.ServiceContent.Setting)
 		opts, err := m.Query(ctx, "config.vpxd.sso.sts.uri")
 		if err == nil && len(opts) == 1 {

@ramineni ramineni changed the title lookup client to use envoy sidecar url if enabled fix: lookup client to use envoy sidecar url if enabled Mar 9, 2024
currently, lookup client iby default fetches the uri from "config.vpxd.sso.sts.uri" value
if service content set . With envoy sidecar enabled, this MR is to
first check for envoy sidecar url if present and use the same.
@ramineni ramineni force-pushed the use-envoy-sidecar-url branch from 302b62d to 8375477 Compare March 9, 2024 02:31
@ramineni
Copy link
Contributor Author

ramineni commented Mar 9, 2024

@dougm Thanks for the review. you are right. Fixed it. Thanks.

Copy link
Member

@dougm dougm left a comment

Choose a reason for hiding this comment

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

Thanks @ramineni

@dougm dougm merged commit 1fb3f62 into vmware:main Mar 11, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants