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

Surface IsNotFound error in GetPreRotationTime #466

Open
aabchoo opened this issue Mar 7, 2025 · 7 comments
Open

Surface IsNotFound error in GetPreRotationTime #466

aabchoo opened this issue Mar 7, 2025 · 7 comments
Labels
good first issue Good for newcomers

Comments

@aabchoo
Copy link
Contributor

aabchoo commented Mar 7, 2025

Description:

GetPreRotationTime should surface apierrors.IsNotFound instead of swallowing the error. Callers of the function should be in charge of handling the error.

if apierrors.IsNotFound(err) {
return time.Time{}, nil

Context:

For OIDC backendSecurityPolicies, the controller is in charge of storing the temporary credentials to access the provider (AWS is only currently supported). The controller does this by storing the credentials (obtained from STS in AWS) in a Kubernetes secret. The secret will be created by the controller if it was previously deleted OR if a new BackendSecurityPolicy was created (or policy was updated to use OIDC).

[optional Relevant Links:]

Any extra documentation required to understand the issue.

This line will need to check if err is NotFound, and if so, rotate credentials.
https://github.com/envoyproxy/ai-gateway/blob/main/internal/controller/backend_security_policy.go#L97

@aabchoo aabchoo added the good first issue Good for newcomers label Mar 7, 2025
@mathetake
Copy link
Member

could you add a note about when that happens? trying to rotate a non-existent secret sounds weird to me, so providing more context will be helpful!

@aabchoo
Copy link
Contributor Author

aabchoo commented Mar 11, 2025

Updated! Let me know if I should add more context 😄

@mathetake
Copy link
Member

looks good and agree it's a good first issue

@teaglebuilt
Copy link

It looks like this has been completed. Can this be closed?

@mathetake
Copy link
Member

how this has been completed? not sure

@teaglebuilt
Copy link

@mathetake must have been an oversight. Can you assign this to me? I am already using this and would like to find a couple things to contribute to. Starting here.

@mathetake
Copy link
Member

sure, feel free to raise a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants