Skip to content

Conversation

@okogut
Copy link
Contributor

@okogut okogut commented Aug 8, 2025

Some companies do not allow all pods to call the nodes api in the cluster,

Currently there is no way to skip the call to the nodes endpoint, which if you do not have the permission will potentially make noise in the logs showing a lot of failed (403 Forbidden) HTTP requests (see below).
K8sClientService.GetNodesAsync ignores the forbidden response, by returning an empty IEnumerable<V1Node>, but the request is still being performed.

This change adds an option to ExcludeNodeInformation, which you can set to true, if you want to skip the call to nodes endpoint entirely - typically because you don't have permission (and likely won't get it) to call the endpoint.

image

@okogut
Copy link
Contributor Author

okogut commented Aug 8, 2025

@microsoft-github-policy-service agree

Copy link
Member

@xiaomi7732 xiaomi7732 left a comment

Choose a reason for hiding this comment

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

It looks largely good to me. Just 1 minor code consistency, please update the code to parse the option value in the constructor in K8sEnvironmentFactory class.

Refer to here as an example

@okogut okogut requested a review from xiaomi7732 August 9, 2025 06:24
@okogut
Copy link
Contributor Author

okogut commented Aug 12, 2025

@xiaomi7732 I updated what you mentioned.

Copy link
Member

@xiaomi7732 xiaomi7732 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for the contributions!

@okogut
Copy link
Contributor Author

okogut commented Aug 12, 2025

A test failed due to missing setup of IOptions<AppInsightsForKubernetesOptions>.Value - fixed now.

@okogut okogut requested a review from xiaomi7732 August 12, 2025 17:45
Copy link
Member

@xiaomi7732 xiaomi7732 left a comment

Choose a reason for hiding this comment

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

LGTM

@xiaomi7732 xiaomi7732 merged commit 0f77494 into microsoft:develop Aug 13, 2025
3 checks passed
@xiaomi7732
Copy link
Member

xiaomi7732 commented Aug 13, 2025

Will be released in 8.0.0-beta2.

@okogut okogut deleted the feature/exclude-node-info branch August 13, 2025 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants