Add KOTS linter as 5th linter with cross-linter improvements #648
Conversation
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
Bishibop
force-pushed
the
feat/improve-multidoc-yaml-handling
branch
from
1cdc6dc to
f00719a
Compare
Bishibop
changed the base branch from
feat/improve-multidoc-yaml-handling
to
feat/embedded-cluster-and-kots-linter-integrations
Implements KOTS Config linting using the kots binary, following the established pattern from the Embedded Cluster linter. Key features: - Added KOTS tool infrastructure (types, config, downloader) - Implemented DiscoverKotsPaths() with GVK filtering (kots.io group) - Created pkg/lint2/kots.go with LintKots() function - Added KOTS result types and LintableResult interface - Integrated KOTS linter into CLI workflow - Supports REPLICATED_KOTS_PATH env var for local binary override - Validates 0-1 KOTS Config per project (errors on multiple) - Includes KOTS in auto-discovery, version display, and summary The KOTS linter uses GVK filtering to distinguish KOTS Config (kots.io/v1beta1 Config) from Embedded Cluster Config (embeddedcluster.replicated.com/v1beta1 Config).
Adds unit tests, integration tests, testdata, and discovery tests for the KOTS linter implementation: - Unit tests (kots_test.go): Tests for message formatting, severity normalization, and JSON parsing with various edge cases - Integration tests (kots_integration_test.go): Tests for actual KOTS binary execution (requires REPLICATED_KOTS_PATH) - Discovery tests (discovery_test.go): Tests for GVK-based discovery that properly distinguishes KOTS Config from EC Config - Testdata files: Various KOTS config scenarios including valid, EC-only, mixed, wrong kind, and malformed apiVersion All unit tests pass. Integration tests require REPLICATED_KOTS_PATH environment variable to be set to a local kots binary. Test coverage: - Message formatting with line numbers and rules - Severity mapping (error/warn/info → ERROR/WARNING/INFO) - JSON parsing with surrounding text - Empty outputs and malformed JSON - GVK filtering edge cases (different versions, wrong kind, etc.) - Multi-document YAML with mixed KOTS and EC configs
…C linters This commit fixes two bugs discovered during manual testing of the KOTS linter: Bug 1: Summary counts always showing 0 (HIGH PRIORITY) ======================================================== Root Cause: Case mismatch between linter severity values and summary calculator - KOTS and EC linters returned lowercase: "error", "warning", "info" - Summary calculator expected uppercase: "ERROR", "WARNING", "INFO" - Result: Summary always showed "0 error(s), 0 warning(s), 0 info" Changes: - pkg/lint2/kots.go: Updated normalizeKotsSeverity() to return uppercase - pkg/lint2/embedded_cluster.go: Updated severity assignments to uppercase - pkg/lint2/kots_test.go: Updated 10 test expectations to uppercase - pkg/lint2/embedded_cluster_test.go: Updated 6 test expectations to uppercase Impact: Summary counts now display correctly for both linters - Valid config: "0 error(s), 3 warning(s), 0 info" ✓ (was 0, 0, 0) - Invalid config: "1 error(s), 3 warning(s), 0 info" ✓ (was 0, 0, 0) Bug 2: Auto-discovery showed contradictory message (LOW PRIORITY) =================================================================== Root Cause: KOTS linter was disabled by default while all others were enabled - Helm, Preflight, SupportBundle, EC: Disabled = false (enabled) - KOTS: Disabled = true (disabled) ← inconsistent - Auto-discovery found KOTS files but didn't lint them - Message: "KOTS linting is disabled in .replicated config" (but no config!) Changes: - pkg/tools/config.go: Changed KOTS default from disabled to enabled (line 272) - pkg/tools/config_test.go: Added 3 comprehensive tests: * TestApplyDefaultsWithNilReplLint - verifies all linters enabled by default * TestKotsLinterDefaultConsistency - prevents KOTS from diverging * TestKotsLinterExplicitDisable - backward compatibility test Impact: KOTS now consistent with other linters - Auto-discovery: KOTS files are linted by default ✓ - Explicit disable: Still works via config ✓ - No breaking changes for existing users ✓ Testing: - All unit tests pass (125+ tests in pkg/lint2) - All config tests pass (30+ tests in pkg/tools) - Manual end-to-end testing verified both fixes - No regressions in existing linters Fixes affect both KOTS and Embedded Cluster linters.
1. Make severity normalization case-insensitive - Changed normalizeKotsSeverity() to use strings.ToLower() - Handles "ERROR", "Error", "WARN", etc. correctly - Added 5 new test cases for case variations - Prevents incorrect severity mapping to INFO 2. Make multi-config handling graceful (match EC pattern) - Changed from hard error to graceful failure - Creates failed results instead of stopping entire lint - Displays errors for all configs - Other linters continue running - Consistent with embedded-cluster linter behavior Both changes improve robustness and user experience.
Adds KOTS to TestGetLatestStableVersion_AllTools to verify that: - The ping API returns a version for KOTS - The version is non-empty - The mapping from ToolKots to 'kots' key works correctly This ensures complete test coverage for the ping API integration across all 5 supported tools (Helm, Preflight, Support Bundle, Embedded Cluster, and KOTS).
This commit includes three improvements to pkg/tools/config.go: 1. Fixed KOTS linter default inconsistency (BUG) - Line 251 in DefaultConfig() was setting KOTS to disabled (true) - Line 272 in ApplyDefaults() was setting KOTS to enabled (false) - This caused different behavior depending on which code path was taken - Fixed by changing line 251 to false (enabled, consistent with other linters) 2. Extracted defaultLintersConfig() helper function - Eliminates duplication between DefaultConfig() and ApplyDefaults() - Single source of truth for linter defaults (prevents future bugs) - Reduces 14 lines to 2 lines in each function (12 lines saved) 3. Simplified tool version defaults loop - Changed from 5 repetitive if-exists checks to a single loop - More maintainable: future tools just add to the array - Clearer intent: "apply latest to all tools" - Reduces 15 lines to 14 lines with better readability All existing tests pass. These changes improve maintainability and prevent the type of bug that was just fixed.
This commit creates a unified binary resolution helper that all 5 linters use, with consistent environment variable override support for local development. Changes: 1. Created pkg/lint2/lint_common.go with resolveLinterBinary() helper - Checks env var first, falls back to resolver - Single source of truth for binary resolution logic - Consistent error handling across all linters 2. Updated all 5 linters to use the unified helper: - helm.go: Added REPLICATED_HELM_PATH support (NEW) - preflight.go: Added REPLICATED_PREFLIGHT_PATH support (NEW) - support_bundle.go: Added REPLICATED_SUPPORT_BUNDLE_PATH support (NEW) - embedded_cluster.go: Refactored to use helper (keeps REPLICATED_EMBEDDED_CLUSTER_PATH) - kots.go: Refactored to use helper (keeps REPLICATED_KOTS_PATH) 3. Each linter reduced from ~10 lines to 3 lines for binary resolution Environment Variables (for local development): - REPLICATED_HELM_PATH - REPLICATED_PREFLIGHT_PATH - REPLICATED_SUPPORT_BUNDLE_PATH - REPLICATED_EMBEDDED_CLUSTER_PATH - REPLICATED_KOTS_PATH Benefits: - 40+ lines of duplicated code → single 55-line helper function - Consistent env var override for ALL linters (not just EC and KOTS) - Single source of truth for binary resolution - Easier to add future linters (just call one function) - Maintains backward compatibility for existing env vars All existing tests pass, including env var override tests for EC and KOTS.
This commit creates a generic helper function to eliminate 62 lines of duplicated business logic for validating single config limits. Changes: 1. Created validateSingleConfigLimit[T LintableResult]() generic function - Uses Go generics to work with both EmbeddedClusterLintResult and KotsLintResult - Takes constructor function for type-safe result creation - Handles validation, error messages, and display in one place 2. Updated lintEmbeddedClusterConfigs() in cli/cmd/lint.go - Replaced 32 lines (743-774) with 23-line helper call - Passes EC-specific type names and constructor 3. Updated lintKotsManifests() in cli/cmd/lint.go - Replaced 32 lines (821-852) with 23-line helper call - Passes KOTS-specific type names and constructor Benefits: - 62 lines → ~35 lines (27 lines saved, more readable) - Business logic centralized (validation rules in one place) - Type-safe through Go generics (compiler catches errors) - Consistent error messages across linters - Testable independently - Future-proof: new linters just call the helper Implementation Notes: - Generic function (not method) because Go doesn't support type parameters on methods - First parameter is *runners to access outputFormat and displayLintResults - Maintains exact same behavior as before (no functional changes) - All existing CLI tests pass This is the second refactoring from KOTS_LINTER_REFACTORING_OPPORTUNITIES.md
- Remove TODO about env var overrides being temporary (they're permanent) - Simplify function documentation to be more concise - Keep comments focused on non-obvious behavior rather than restating code
Enhance error messages when multiple EC or KOTS configs are found by adding clear guidance on how to resolve the issue. Users now see: "Remove duplicate configs or specify a single config file" This applies to both Embedded Cluster and KOTS linters via the generic validateSingleConfigLimit() function.
Quick wins for code readability: - Rename err → cmdErr at command execution sites for clarity - Rename appResolveErr → appResolutionWarning to reflect usage as warning - Update resolveAppContext comment to clarify return value semantics - Add explicit documentation for validateSingleConfigLimit return values - Make success detection more explicit with better variable names Changes: - All 5 linters: Changed `err` to `cmdErr` and `success` to `lintSuccess` for clearer connection between command execution and success detection - cli/cmd/lint.go: Renamed `appResolveErr` to `appResolutionWarning` (used for warnings, not fatal errors) - Added detailed return value documentation for `resolveAppContext()` and `validateSingleConfigLimit()` to clarify behavior No behavior changes - documentation and naming improvements only.
Replace verbose embedded-cluster section (148 lines) with comprehensive "Linter-Specific Configuration" section covering all 5 linters (135 lines). Each linter now has consistent documentation: - Helm: Chart structure and template validation - Support Bundle: Collector spec validation - Embedded Cluster: EC config validation (trimmed from 148 to ~30 lines) - KOTS: KOTS Config manifest validation (new, ~40 lines) All sections include: - What it validates - Configuration example - Auto-discovery behavior - Key constraints (e.g., 1 config limit for EC and KOTS) KOTS section highlights: - GVK filtering distinguishes KOTS Config (kots.io) from EC Config - Multiple config error message with actionable guidance - Example KOTS Config manifest Removed platform requirements for EC (Darwin support coming soon).
Bishibop
force-pushed
the
feat/add-kots-linter
branch
from
ccddeb1 to
973cbfe
Compare
Add comprehensive examples for the KOTS linter: - examples/kots/valid-config.yaml: Complete KOTS Config with multiple groups, field types (text, password, bool, select_one), defaults, and help text - examples/kots/invalid-config.yaml: Intentionally broken config for testing error handling (invalid types, malformed templates, security issues) - examples/kots/README.md: Documentation explaining examples, GVK filtering, single config limit, and usage instructions - examples/.replicated.yaml: Updated to include KOTS linter configuration Follows the pattern established by embedded-cluster examples.
bennyyang11
approved these changes
Nov 11, 2025
Contributor
bennyyang11
left a comment
bennyyang11
left a comment
There was a problem hiding this comment.
KOTS linter works but there is a bug with preflight. Fix will be implemented after we merge all the EC and KOTS linter implementations.
| disabled: false | ||
| embedded-cluster: | ||
| disabled: false | ||
| kots: |
Contributor
There was a problem hiding this comment.
When testing it seems like preflight was broken so you must disabled: true for preflights. It will just stop once it hits preflights and not continue to lint support-bundle, EC, and KOTS. Once disabled for preflights, it will run all the way through and KOTS works.
Bishibop
merged commit 31b7138
into
feat/embedded-cluster-and-kots-linter-integrations
4 checks passed
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.
Summary
Adds KOTS Config linting as the 5th linter alongside Helm, Preflight, Support Bundle, and Embedded Cluster.
Key Features
kind: ConfigwithapiVersion: kots.io/*Cross-Linter Improvements
This PR includes refactoring that benefits all 5 linters:
Documentation
Added balanced linter sections for all 5 linters in
docs/lint-format.mdwith consistent structure.