feat(lint): custom action framework linter#1789
Draft
osmman wants to merge 4 commits into
Draft
Conversation
Replace StatusUpdate(ctx, obj) *Result with PersistStatus(ctx, obj) (bool, error) that only persists status without controlling reconciler flow. Developers now explicitly choose flow control after persisting. The old StatusUpdate always called Status().Update() and returned a *Result, conflating persistence with flow control. When the status was unchanged, it still wrote to the API server. The new PersistStatus skips the API call when status is unchanged (no-op optimization) and returns a changed flag so callers can react appropriately: - changed=true: API write happened, watch event will fire → use Return() - changed=false: no write, no watch event → use Requeue() as safety net Migration applied to all ~124 call sites across 60+ action files. Other changes: - Add RequeueAfter(delay) for explicit polling delays - Change Requeue() default from 5s to 100ms (safety-net, not polling) - Polling sites now use RequeueAfter(5 * time.Second) explicitly - Fix missing return before Error() in rekor sharding_config.go - Fix tuf resolve_keys.go: persist status before Continue() - Add package-level doc.go with patterns and anti-patterns - Add PersistStatus test coverage including conflict retry - Add godoc to all BaseAction methods and Action interface Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Tomas Turek <tturek@redhat.com>
Extract custom config handling into handleCustomConfig method to bring cyclomatic complexity below the gocyclo threshold of 30. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add two analyzers as a separate Go module (pkg/analyzer/) integrated via golangci-lint v2 module plugin: - persiststatuserr: flags PersistStatus() calls where the error is discarded - specmutation: flags Spec field mutations inside action types CI workflow updated to build the custom golangci-lint binary and use it with golangci-lint-action for inline PR annotations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- ctlog: extract resolveTrillianAddress() helper to avoid mutating instance.Spec.Trillian.Address when defaulting empty values - fulcio: write calculated hostname directly to Status.Certificate.CommonName instead of Spec.ExternalAccess.Host Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
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
1 participant
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
PersistStatus()calls where the error return is discardedinstance.Specmutations inside action types (only Status is persisted)pkg/analyzer/) to keep operator deps cleangolangci-lint-actionwithinstall-mode: nonefor inline PR annotationsTest plan
go test ./pkg/analyzer/...)act🤖 Generated with Claude Code