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

Conversation

dehaansa
Copy link
Contributor

@dehaansa dehaansa commented Nov 21, 2024

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: vmware/govmomi#1200 .

Link to tracking issue

Related issue in Grafana Alloy: grafana/alloy#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.

@dehaansa dehaansa requested review from djaglowski and a team as code owners November 21, 2024 07:08
Copy link

linux-foundation-easycla bot commented Nov 21, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@schmikei
Copy link
Contributor

image

I've tested with a variety of TLS settings (using mitmproxy) and things seem good to me and based off those related issues it does seem like underlying client TLS settings should be set this way.

Copy link
Contributor

@BominRahmani BominRahmani left a comment

Choose a reason for hiding this comment

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

Nice work!

@djaglowski djaglowski merged commit a6fa27b into open-telemetry:main Dec 3, 2024
158 checks passed
@github-actions github-actions bot added this to the next release milestone Dec 3, 2024
@dehaansa dehaansa deleted the fix/vcenter-tls branch December 3, 2024 19:57
shivanthzen pushed a commit to shivanthzen/opentelemetry-collector-contrib that referenced this pull request Dec 5, 2024
…ent call (open-telemetry#36482)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### 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:
vmware/govmomi#1200 .

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Related issue in Grafana Alloy:
grafana/alloy#193

<!--Describe what testing was performed and which tests were added.-->
#### Testing
~~This has not been tested, I would appreciate the assistance of any
codeowner that could test.~~ See comments on the PR for test.

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
ZenoCC-Peng pushed a commit to ZenoCC-Peng/opentelemetry-collector-contrib that referenced this pull request Dec 6, 2024
…ent call (open-telemetry#36482)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### 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:
vmware/govmomi#1200 .

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Related issue in Grafana Alloy:
grafana/alloy#193

<!--Describe what testing was performed and which tests were added.-->
#### Testing
~~This has not been tested, I would appreciate the assistance of any
codeowner that could test.~~ See comments on the PR for test.

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
…ent call (open-telemetry#36482)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### 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:
vmware/govmomi#1200 .

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Related issue in Grafana Alloy:
grafana/alloy#193

<!--Describe what testing was performed and which tests were added.-->
#### Testing
~~This has not been tested, I would appreciate the assistance of any
codeowner that could test.~~ See comments on the PR for test.

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
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.

5 participants