Fix: improve operator robustness, performance and code quality#39
Open
enriqueavindi wants to merge 11 commits intomainfrom
Open
Fix: improve operator robustness, performance and code quality#39enriqueavindi wants to merge 11 commits intomainfrom
enriqueavindi wants to merge 11 commits intomainfrom
Conversation
…rcommit logic - Return safe no-op values (1.0, 1.0) when errors occur instead of continuing with zero values or crashing on nil pointer dereference - Remove unused getNamespaceYAML function and YAML roundtrip - Properly handle all error paths in getNamespaceOvercommit and checkOvercommitType to avoid mutating pods with incorrect values - Use direct client.Get for namespace instead of YAML marshal/unmarshal
- Add annotation 'overcommit.inditex.dev/applied' to track if a pod has already been mutated by the webhook - Skip mutation on reinvocation (reinvocationPolicy=IfNeeded) if pod was already processed by the same overcommit class - Store applied CPU/memory ratios in annotations for observability - Resize operations always re-apply since limits may have changed
- Remove RequeueAfter=10s from both Overcommit and OvercommitClass reconcilers to reduce unnecessary API server load - Rely on event-driven reconciliation (watches) which is the standard controller-runtime pattern for reacting to resource changes
- Accept context parameter in Overcommit and OvercommitOnResize functions, propagated from the webhook admission handler - Accept context in GetDefaultSpec and GetPodServiceAccount instead of creating context.Background()/context.TODO() internally - Enables proper cancellation, timeouts, and distributed tracing through the full call chain
- Remove ~30 getter/setter methods that simply forwarded to the embedded metav1.ObjectMeta, which already implements the v1.Object interface via Go struct embedding - Remove unused 'k8s.io/apimachinery/pkg/types' import - Reduces ~140 lines of dead code improving maintainability
- Limit regex length to 512 characters to mitigate catastrophic backtracking (ReDoS) attacks via overly complex patterns - Improve error message to include the actual regexp compilation error for better debugging
- Change certificate duration from 87600h (10y) to 8760h (1y) - Shorter-lived certificates reduce the attack surface if a private key is compromised, following security best practices - cert-manager will still auto-renew 30 days before expiry
ismaelpuerto
approved these changes
Mar 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixs #38
This PR addresses several issues found during a code review of the operator
Checklist
git commit -S)