refactor: apply best practices across controller, tests, CI, and Helm chart#321
Merged
refactor: apply best practices across controller, tests, CI, and Helm chart#321
Conversation
… chart Replace global mutable state with an injected Config struct, eliminating data races in parallel tests and improving testability. Introduce sentinel errors (ErrWrongSecretType, ErrWrongSecretKey, ErrInvalidKubeConfig) for programmatic error handling with errors.Is(). Reconciler improvements: - Fix potential panic on empty secretList by extracting deleteArgoSecretByLabels helper - Add watch predicates to filter only *-kubeconfig secrets, reducing reconciliation load - Handle missing CAPI CRDs gracefully with apimeta.IsNoMatchError - Use passed ctx consistently instead of context.Background() - Replace switch/case bool with idiomatic if/else - Fix sync-duration flag description typo - Add custom Prometheus metrics (created/updated/deleted counters) Testing: - Remove deprecated Ginkgo v1 / Gomega, replace with TestMain + envtest - Fix data races by passing Config as parameter instead of mutating globals - Use errors.Is() assertions instead of string comparison - Add tests for sentinel errors and ValidateCapiNaming - Rename constructs_test.go to testhelpers_test.go CI/CD: - Enable golangci-lint in CI workflow - Add pull_request trigger to CI and CodeQL workflows - Add weekly schedule to CodeQL for continuous security scanning - Update Go image to 1.24 and golangci-lint to v2.1.6 Helm chart: - Add PodDisruptionBudget template - Add NetworkPolicy template with DNS/API server egress rules - Add readOnlyRootFilesystem to container security context defaults
golangci-lint-action v6 does not support golangci-lint v2.x. Bump the action to v7 which is required for v2.1.6. Signed-off-by: dntosas <[email protected]>
Code fixes: - Add blank line after embedded field in Capi2Argo struct (embeddedstructfieldcheck) - Move unexported deleteArgoSecretByLabels after exported SetupWithManager (funcorder) - Add period at end of multi-line comment block (godot) - Add blank lines before return statements in test closures (nlreturn) - Fix cuddled assignments and if-statements (wsl) Linter config (.golangci.yml): - Disable gochecknoinits: init() is the standard pattern for scheme/metrics registration - Disable testpackage: tests access unexported functions intentionally - Disable funlen: table-driven tests and main() exceed the 60-line default - Disable maintidx: k8s controller Reconcile functions are inherently complex - Raise cyclop/gocyclo/gocognit thresholds for Reconcile function complexity - Raise nestif threshold for table-driven test blocks Signed-off-by: dntosas <[email protected]>
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
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.
Replace global mutable state with an injected Config struct, eliminating
data races in parallel tests and improving testability. Introduce sentinel
errors (ErrWrongSecretType, ErrWrongSecretKey, ErrInvalidKubeConfig) for
programmatic error handling with errors.Is().
Reconciler improvements:
Testing:
CI/CD:
Helm chart: