Add Embedded Cluster Linter Integration #643
Merged
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
…tion Add infrastructure for embedded cluster config linting, with stub implementation pending real embedded-cluster lint command availability. Changes: - Add EC config discovery with GVK-based filtering (embeddedcluster.replicated.com) - Integrate EC linter into CLI lint command - Add EC tool downloader supporting Linux amd64 only - Implement stub linter returning success until real implementation ready - Simplify GVK wrapper functions for consistency across all discovery Validation: - Enforce maximum 1 EC config per project - Validate config file exists before linting TODOs for future PRs: - Replace stub with actual embedded-cluster lint command execution - Add comprehensive discovery and integration test coverage - Use build-time CLI version instead of hardcoded "v0.90.0"
- Remove duplicate env var calls (REPLICATED_API_TOKEN and REPLICATED_API_ORIGIN already set in root.go) - Add non-TTY detection for interactive app selection prompt to prevent failures in CI/non-interactive environments - Improve error handling for nil appObj with clearer messages - Document environment variables available to embedded-cluster binary (REPLICATED_APP, REPLICATED_API_TOKEN, REPLICATED_API_ORIGIN) Addresses issues identified in code review of origin/ec-lint-auth
Previously, app context (REPLICATED_APP env var) was only resolved and set when the embedded-cluster linter was enabled. This refactoring makes app resolution run unconditionally for ALL linters, providing vendor portal context to all linter binaries. Changes: - Extract app resolution logic into resolveAppContext() function - Move app resolution out of embedded-cluster-specific conditional - Make app resolution graceful: returns empty string if no app available (except when user explicitly specifies an app that can't be resolved) - Update docs to note env vars available to all linters, not just EC This future-proofs the linting infrastructure for upcoming features that will require vendor portal integration across all linters. Resolution priority: 1. runners.appID (from --app-id flag or REPLICATED_APP env) 2. runners.appSlug (from --app-slug flag) 3. config.AppId (from .replicated config) 4. config.AppSlug (from .replicated config) 5. API fetch (auto-select if 1 app, prompt if multiple, graceful if 0)
…rrors Previously, if app context resolution failed, the entire lint command would fail, preventing all linters from running. This changes the behavior to: 1. Store app resolution errors instead of returning immediately 2. Continue running other linters (helm, preflight, support-bundle) 3. Only fail the embedded-cluster linter with an error result 4. Display the app resolution error in the EC linter's output This ensures that helm/preflight/support-bundle linting can proceed even when app context is unavailable, while embedded-cluster linting properly reports why it cannot run. Example error displayed in EC linter output: ERROR: Embedded cluster linting requires app context: failed to resolve app from selection: my-app (app not found or invalid)
Adds comprehensive integration tests for app context resolution logic that use real API calls to the vendor portal. Tests are tagged with //go:build integration and only run when explicitly requested. Run with: go test -tags=integration -v ./cli/cmd -run TestResolveAppContext Prerequisites: - REPLICATED_API_TOKEN environment variable - REPLICATED_API_ORIGIN (optional, defaults to api.replicated.com) - At least one app in the vendor account Test coverage: - App resolution with no app specified (API auto-select) - Explicit app ID in config - Explicit app slug in config - App ID set via runners (--app-id flag) - Invalid app ID error handling - Priority order verification (runners.appID > config.AppId) These tests verify the production behavior with real vendor portal APIs, avoiding the complexity of mocking the 100+ method VendorV3Client.
Replaces stub implementation with actual embedded-cluster lint execution. Changes: - Add support for REPLICATED_EMBEDDED_CLUSTER_PATH env var for local development - Execute embedded-cluster lint with --output json flag - Parse JSON output and convert to LintMessage format - Handle both success and error cases (errors/warnings/infos) - Extract JSON from output when followed by error messages The linter inherits REPLICATED_APP, REPLICATED_API_TOKEN, and REPLICATED_API_ORIGIN environment variables set by the caller for vendor portal API integration. Tested with: - Valid configs (returns success with no messages) - Invalid configs with YAML syntax errors (returns errors)
Extracts common JSON extraction logic into a shared extractJSONFromOutput()
function that all linters (preflight, support-bundle, embedded-cluster) can use.
Changes:
- Add extractJSONFromOutput() to troubleshoot_common.go
- Searches for each '{' in output
- Uses brace-counting to find complete JSON objects
- Validates each candidate as JSON
- Handles error messages before/after JSON (e.g., "Error: {invalid}" or "ERROR: validation failed")
- Update parseTroubleshootJSON() to use shared function (reduced from ~30 to ~10 lines)
- Update parseEmbeddedClusterOutput() to use shared function (reduced from ~50 to ~15 lines)
- Move file existence check before binary resolution in LintEmbeddedCluster (faster, fixes test on darwin-arm64)
Benefits:
- Removes 50+ lines of duplicate brace-counting code
- More robust: combines search-and-try with brace-counting validation
- Easier to test: single function for edge cases
- Easier to maintain: future linters can reuse same logic
All tests pass.
… linter Adds comprehensive test coverage for Priority 1 functionality: JSON Parsing Tests (7 tests): - Parse errors only, warnings only, infos only - Parse mixed severities (errors + warnings + infos) - Handle empty output gracefully - Error on malformed JSON - Extract JSON from output with trailing error text (e.g., "ERROR: validation failed") Binary Override Tests (2 tests): - REPLICATED_EMBEDDED_CLUSTER_PATH env var overrides resolver - Falls back to resolver when env var not set - Tests use mock shell script binary returning valid JSON All tests pass. Foundation for additional integration tests.
Adds 5 integration tests that execute real embedded-cluster binary: 1. TestEmbeddedClusterLint_Integration - validates lint with valid/invalid configs 2. TestEmbeddedClusterLint_EnvironmentVariables - verifies env vars accessible 3. TestEmbeddedClusterDiscovery_SingleConfig - discovers single EC config 4. TestEmbeddedClusterDiscovery_MultipleConfigs - discovers multiple configs 5. TestEmbeddedClusterDiscovery_NoConfigs - handles no configs gracefully Tests use //go:build integration tag and require REPLICATED_EMBEDDED_CLUSTER_PATH pointing to local binary. All tests pass (4 PASS, 1 SKIP when no API token). Covers Priority 1 test gaps: config discovery, real binary execution, and end-to-end validation.
Removes GitHub API fallback now that embedded-cluster is available in the replicated.app/ping response (client_versions.embedded_cluster). Changes: - Updated getLatestStableVersion() to use "embedded_cluster" key from ping API - Removed getLatestEmbeddedClusterVersion() function and githubRelease struct - Added integration tests to verify ping API returns EC version - Added unit test to validate version key mapping for all tools Benefits: - Eliminates GitHub API rate limit dependency (60/hour unauthenticated) - Consistent with other tools (helm, preflight, support-bundle) - Simpler code (-36 lines, no special case needed) - More reliable (ping API is source of truth for all tool versions) Test Results: - TestGetLatestStableVersion_EmbeddedCluster: PASS (returns 2.12.0+k8s-1.33) - TestGetLatestStableVersion_AllTools: PASS (all 4 tools) - TestGetVersionKey: PASS - All existing tests: PASS
…linters Previously, if 2+ embedded-cluster configs were found, the entire lint command would fail with a hard error, blocking helm, preflight, and support-bundle linters from running. Changes: - Moved validation from extractAllPathsAndMetadata() to lintEmbeddedClusterConfigs() - EC linter now returns failed results (not fatal error) when 2+ configs found - Other linters (helm, preflight, support-bundle) continue running normally - Follows same graceful error handling pattern as app resolution errors Test Coverage: - Added TestEmbeddedClusterLint_MultipleConfigsGracefulFailure - Verifies EC linter returns results (not fatal error) - Confirms clear error message: "Multiple embedded cluster configs found" - Validates all configs show Success: false - Documents that other linters would continue Benefits: - User gets all linting results, not just EC error - Consistent with other graceful failure patterns - Better developer experience (more actionable feedback)
- Replace hardcoded CLI version with version.Version() - Add comprehensive embedded-cluster documentation to lint-format.md - Create examples/embedded-cluster/ with valid/invalid configs - Document REPLICATED_EMBEDDED_CLUSTER_PATH as temporary dev workaround This completes the documentation and polish tasks for the EC linter feature.
Add 14 comprehensive test cases to verify Group-Version-Kind (GVK) filtering correctly distinguishes between Embedded Cluster Config and KOTS Config. Key tests added: - KOTS Config rejection (kots.io/v1beta1) when searching for EC Config - EC Config acceptance with multiple API versions (v1beta1, v1beta2, v1) - Group case sensitivity (embeddedcluster.replicated.com exact match) - Kind case sensitivity (Config exact match) - Missing/empty apiVersion handling - Non-Config EC kinds rejection (Installation, ClusterProfile, etc.) - Multi-document YAML with mixed config types - Empty files and comments-only files - Malformed apiVersion formats These tests close a critical gap in test coverage - the existing tests covered generic GVK matching but not the crucial EC vs KOTS distinction. All 14 new tests pass, and all existing tests remain passing.
When app context resolution fails (e.g., multiple apps, no --app-id in CI/CD) and the embedded-cluster linter is enabled, immediately warn the user that EC linting will fail but other linters will continue. This provides better UX by: - Informing users upfront instead of waiting for EC results - Making it clear that helm/preflight/support-bundle will still run - Reducing confusion about why EC linter failed Example output:⚠️ Warning: Could not resolve app context: multiple apps available... Embedded-cluster linter will fail. Other linters will continue. HELM CHARTS ✓ chart1: Passed ... EMBEDDED CLUSTER ✗ ec-config.yaml: Failed ERROR: Embedded cluster linting requires app context: ...
Fixes nil pointer dereference in unit tests where kotsAPI is not initialized. The resolveAppContext function now checks if r.kotsAPI is nil before calling ListApps(), preventing panics in test environments. Additionally, the warning message for app resolution failures is now only displayed in table output mode, preventing interference with JSON parsing. Changes: - Add nil check before r.kotsAPI.ListApps() call - Restrict warning output to table mode only (not JSON mode) This allows tests that don't need API access to run without initializing the full API client infrastructure. Fixes: TestJSONOutputContainsAllToolVersions
Bishibop
changed the base branch from
main
to
feat/embedded-cluster-and-kots-linter-integrations
Bishibop
merged commit ee00c41
into
feat/embedded-cluster-and-kots-linter-integrations
5 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.
Add Embedded Cluster Linter Integration
Summary
Adds support for linting Embedded Cluster configuration files (
kind: Config,apiVersion: embeddedcluster.replicated.com/v1beta1) to thereplicated lintcommand.IMPORTANT
Since EC has not cut releases with the linter, this PR just uses a local build of the linter for now.
Setup
Navigate to your replicated repo
Navigate to your embedded-cluster repo
cd /path/to/embedded-clusterBuild the binary (requires the lint command)
make buildPoint to your local embedded-cluster binary
export REPLICATED_EMBEDDED_CLUSTER_PATH="/path/to/embedded-cluster/output/bin/embedded-cluster"Set your Replicated API token (required for app context)
export REPLICATED_API_TOKEN="your-token-here"Features
kind: Config)Key Implementation Details
GVK Filtering
Solves collision between EC Config and KOTS Config (both use
kind: Config):embeddedcluster.replicated.com/v1beta1+Configkots.io/v1beta1+ConfigdiscoverYAMLsByGVK()Refactoring
Error Handling