Skip to content
This repository was archived by the owner on Sep 11, 2025. It is now read-only.

Conversation

@mattjohnsonpint
Copy link
Contributor

@mattjohnsonpint mattjohnsonpint commented Jul 12, 2025

When initializing the kubernetes secretes provider, we should timeout if there are errors, rather than waiting indefinitely.

Some refactoring included.

@mattjohnsonpint mattjohnsonpint requested review from a team and Copilot July 12, 2025 17:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a timeout for the Kubernetes secret provider’s cache synchronization and refactors client initialization.

  • Introduce a cacheSyncTimeout constant and wrap cache.WaitForCacheSync with a context timeout
  • Move ctrl.SetLogger call into initialize and update newK8sClientForSecret to use ctrl.GetConfig with error handling
  • Update CHANGELOG with the new fix entry

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
runtime/secrets/kubernetes.go Implemented cache sync timeout and refactored client initialization
CHANGELOG.md Added entry for cache sync timeout fix
Comments suppressed due to low confidence (2)

runtime/secrets/kubernetes.go:31

  • Add a comment explaining the rationale for the 10-second cache sync timeout and how to adjust it if needed.
var cacheSyncTimeout = 10 * time.Second

runtime/secrets/kubernetes.go:91

  • Add tests to simulate cache sync timeout and verify that timeout errors are handled correctly.
	waitCtx, cancel := context.WithTimeout(ctx, cacheSyncTimeout)

@mattjohnsonpint mattjohnsonpint enabled auto-merge (squash) July 12, 2025 17:08
@mattjohnsonpint mattjohnsonpint merged commit d3db048 into main Jul 12, 2025
32 of 33 checks passed
@mattjohnsonpint mattjohnsonpint deleted the mjp/fix-k8s-loop branch July 12, 2025 17:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants