Skip to content

feat: CSI Token Requests#561

Open
ThirdEyeSqueegee wants to merge 3 commits intomainfrom
tokens
Open

feat: CSI Token Requests#561
ThirdEyeSqueegee wants to merge 3 commits intomainfrom
tokens

Conversation

@ThirdEyeSqueegee
Copy link
Member

@ThirdEyeSqueegee ThirdEyeSqueegee commented Jan 30, 2026

Issue #, if available:
#400, #422

Description of changes:
Implement CSI Token Requests

Testing: e2e tested by hand (rotation not yet tested, awaiting upstream driver release)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ThirdEyeSqueegee ThirdEyeSqueegee requested a review from a team as a code owner January 30, 2026 23:48
@codecov
Copy link

codecov bot commented Jan 30, 2026

Codecov Report

❌ Patch coverage is 95.29412% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.49%. Comparing base (f0808bf) to head (9e19467).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
server/server.go 86.20% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #561      +/-   ##
==========================================
- Coverage   61.56%   61.49%   -0.08%     
==========================================
  Files          11       12       +1     
  Lines         752      748       -4     
==========================================
- Hits          463      460       -3     
+ Misses        274      270       -4     
- Partials       15       18       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ThirdEyeSqueegee ThirdEyeSqueegee added the safe-to-test Pull Request has been manually reviewed and deemed to be safe to run integration tests on. label Jan 30, 2026
@github-actions github-actions bot removed the safe-to-test Pull Request has been manually reviewed and deemed to be safe to run integration tests on. label Feb 2, 2026
@ThirdEyeSqueegee ThirdEyeSqueegee added safe-to-test Pull Request has been manually reviewed and deemed to be safe to run integration tests on. and removed safe-to-test Pull Request has been manually reviewed and deemed to be safe to run integration tests on. labels Feb 2, 2026
@github-actions github-actions bot removed the safe-to-test Pull Request has been manually reviewed and deemed to be safe to run integration tests on. label Feb 2, 2026
@ThirdEyeSqueegee ThirdEyeSqueegee added the safe-to-test Pull Request has been manually reviewed and deemed to be safe to run integration tests on. label Feb 2, 2026
@simonmarty simonmarty added safe-to-test Pull Request has been manually reviewed and deemed to be safe to run integration tests on. and removed safe-to-test Pull Request has been manually reviewed and deemed to be safe to run integration tests on. labels Feb 3, 2026
)
}

if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we log here?

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
k8sClient := fake.NewClientset().CoreV1()
timeout := 100 * time.Millisecond
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the change here?

}

pod_identity := PodIdentityCredentialProvider{
klog.V(2).Infof("Pod Identity token obtained for audience %q", podIdentityAudience)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the .V(2) needed here?

case "ipv6":
return "ipv6"
default:
return "auto" // Default to auto for invalid preferences
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was the comment removed here? Was it inaccurate?


if preference == "auto" || preference == "ipv4" {
config, err := p.getConfigWithEndpoint(ctx, podIdentityAgentEndpointIPv4)
cfg, err := p.getConfigWithEndpoint(ctx, podIdentityAgentEndpointIPv4)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this variable renamed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe-to-test Pull Request has been manually reviewed and deemed to be safe to run integration tests on.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants