From 91e773a438adea5334c6a932408a998c05129b40 Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Tue, 24 Jun 2025 10:13:32 -0400 Subject: [PATCH 01/11] add cursor rules --- .cursor/rules/api-patterns.mdc | 68 ++++++++++++++++++ .cursor/rules/go-testing.mdc | 121 +++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+) create mode 100644 .cursor/rules/api-patterns.mdc create mode 100644 .cursor/rules/go-testing.mdc diff --git a/.cursor/rules/api-patterns.mdc b/.cursor/rules/api-patterns.mdc new file mode 100644 index 0000000000..f85e531bc1 --- /dev/null +++ b/.cursor/rules/api-patterns.mdc @@ -0,0 +1,68 @@ +--- +description: +globs: +alwaysApply: false +--- +# API Design Patterns + +Follow these established patterns for consistent API design and error handling. + +## Error Handling + +**Use Typed Errors Instead of String Matching** +- Use `errors.Is()` for error type checking instead of string comparisons +- Define custom error variables for domain-specific errors +- Error handling should be based on error types, not string content +- Someone may change error messaging and unintentionally affect status codes +- Follow established patterns for error type checking in the codebase + +**Return Consistent Error Responses** +- Always use `types.APIError` for structured error responses +- Use proper error constructor functions: `NewBadRequestError()`, `NewInternalServerError()`, `NewUnauthorizedError()`, etc. +- Let `jsonError()` helper automatically handle error type conversion +- Use `AppendFieldError()` for validation errors with specific field names + +## Handler Structure + +**Follow the Standard Handler Pattern** +- Use `bindJSON()` for request parsing with automatic error handling +- Use `json()` for successful responses with proper content-type headers +- Use `jsonError()` for error responses with consistent formatting +- Use `logError()` for structured logging with request context +- Return early on errors - don't continue processing after `bindJSON()` or other errors + +**Keep HTTP Layer Separate from Business Logic** +- Handlers should be thin - delegate complex logic to controllers +- Controllers handle business logic and return domain errors +- API layer converts domain errors to HTTP responses using typed errors +- Follow the controller pattern: handlers → controllers → managers → stores + +## Dependency Injection & Architecture + +**Use the Option Pattern for Constructors** +- All major components use functional options: `NewController(opts ...Option)` +- Options modify struct fields via closures: `func(c *Controller) { c.field = value }` +- Provide default implementations when nil dependencies are detected +- Use dependency injection to enable testing with mocks + +**Follow the Layered Architecture** +- **API Layer**: HTTP handlers, routing, middleware +- **Controller Layer**: Business logic orchestration, error handling +- **Manager Layer**: Domain-specific operations, workflow management +- **Store Layer**: Data persistence, in-memory or disk-based storage + +## Authentication & Authorization + +**Use Bearer Token Pattern** +- All protected routes use `authMiddleware` on subrouters +- Token extraction follows: `Authorization: Bearer ` +- Use typed errors for auth failures: `NewUnauthorizedError()` +- Authentication logic is centralized in auth controller + +## Route Organization + +**Group Related Routes Using Subrouters** +- Public routes go directly on main router +- Protected routes use `authenticatedRouter` subrouter with middleware +- Domain-specific routes use prefixed subrouters: `/install`, `/console` +- RESTful patterns: GET for retrieval, POST for actions/creation diff --git a/.cursor/rules/go-testing.mdc b/.cursor/rules/go-testing.mdc new file mode 100644 index 0000000000..e19c73a9f1 --- /dev/null +++ b/.cursor/rules/go-testing.mdc @@ -0,0 +1,121 @@ +--- +description: +globs: +alwaysApply: false +--- +# Go Testing Standards + +Follow these testing patterns for Go code to create valuable, maintainable tests. + +## Test Types and Strategy + +**Unit Tests** +- Test individual functions/methods in isolation +- Mock external dependencies +- Focus on business logic and edge cases +- Should be fast and not require external resources + +**Integration Tests** +- Test component interactions +- Use real implementations where feasible +- Place in appropriate directories (`integration/`, `*_integration_test.go`) +- May use external resources but should be self-contained + +## Test Value and Purpose + +**Focus on Behavioral Testing** +- Verify actual function behavior and business logic, not just object instantiation +- Test the contract and expected outcomes of functions +- Ensure tests validate meaningful functionality rather than trivial operations +- Include realistic scenarios that represent actual use cases + +**Comprehensive Test Coverage** +- Test both success and failure paths +- Include edge cases and boundary conditions +- Test error conditions explicitly with proper error type validation + +## Test Organization and Structure + +**Use Table-Driven Tests for Multiple Scenarios** +- Structure tests with `tests := []struct{}` pattern for comprehensive coverage +- Use descriptive test names that explain the scenario being tested +- Group related test cases logically within table-driven tests +- Follow the Arrange-Act-Assert pattern within each test case + +**Test Setup and Cleanup** +- Use `t.TempDir()` for temporary directories - it handles cleanup automatically +- Use `t.Cleanup()` for custom cleanup functions when needed +- Use `defer` statements for resource cleanup within test functions +- Prefer `t.Setenv()` over manual environment variable manipulation + +## Assertion and Error Testing + +**Use Testify Consistently** +- Use `require.NoError(t, err)` for critical assertions that should stop test execution +- Use `assert.NoError(t, err)` for non-critical assertions that allow test to continue +- Use `require.True(t, ok)` for type assertions and critical boolean checks +- Use `assert.Equal()`, `assert.Contains()`, `assert.Empty()` for value comparisons + +**Error Testing Best Practices** +- Test specific error types, not just that an error occurred +- Use `errors.Is()` and `errors.As()` for error checking in tests +- Test error messages when they're part of the API contract +- Test error propagation through the call stack + +## Mock Usage Guidelines + +**When to Use Mocks** +- Mock external dependencies, not the code under test +- Use interface-based mocking for better testability +- Consider using real implementations for lightweight dependencies +- Mock at the right level of abstraction + +**How to Use Mocks Effectively** +- Prefer dependency injection to enable clean mocking +- Set up mock expectations that match real-world scenarios using `mock.InOrder()` when sequence matters +- Don't mock simple value objects or data structures +- Always call `mockObject.AssertExpectations(t)` to verify mock usage + +## Specialized Testing Patterns + +### HTTP and API Testing + +**Use httptest for HTTP Testing** +- Use `httptest.NewServer()` for integration-style HTTP tests +- Use `httptest.NewRequest()` and `httptest.NewRecorder()` for handler unit tests +- Test both request handling and response formatting +- Verify HTTP status codes, headers, and response bodies + +**API Client Testing** +- Test both successful and error response scenarios +- Verify request construction (method, path, headers, body) +- Test error handling and proper error type conversion +- Use `defer server.Close()` to clean up test servers + +### Kubernetes and Controller Testing + +**Use envtest for Kubernetes Testing** +- Use `envtest.Environment` for testing Kubernetes controllers +- Use `fake.NewClientBuilder()` for lightweight client testing +- Set up proper schemes with `AddToScheme()` calls +- Use `t.Cleanup()` to stop test environments + +**Controller Runtime Testing Patterns** +- Create realistic Kubernetes objects for testing +- Test both creation and update scenarios +- Verify object state changes and status updates +- Use proper context handling with `logr.NewContext()` + +## Test Data and Resource Management + +**Keep Test Data Clean** +- Use test-specific data that doesn't depend on external state +- Create test data within test functions or table-driven test cases +- Use embedded test files when appropriate (`//go:embed`) +- Avoid shared mutable state between tests + +**Temporary Resources** +- Use `t.TempDir()` for file-based operations +- Clean up network resources (listeners, servers) with defer statements +- Use unique names/ports to avoid conflicts between parallel tests +- Create minimal test fixtures that focus on the specific test scenario \ No newline at end of file From 24c3dc8a0e0d5ab7322e38739e05c7412db74d24 Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Sun, 29 Jun 2025 10:21:43 -0400 Subject: [PATCH 02/11] cursor rules --- .cursor/rules/api-patterns.mdc | 68 ----- .cursor/rules/go-best-practices.mdc | 223 ++++++++++++++ .cursor/rules/go-ec-api-architecture.mdc | 96 ++++++ .cursor/rules/go-testing.mdc | 279 ++++++++++-------- .../embedded-cluster-operator/values.yaml | 3 + 5 files changed, 485 insertions(+), 184 deletions(-) delete mode 100644 .cursor/rules/api-patterns.mdc create mode 100644 .cursor/rules/go-best-practices.mdc create mode 100644 .cursor/rules/go-ec-api-architecture.mdc diff --git a/.cursor/rules/api-patterns.mdc b/.cursor/rules/api-patterns.mdc deleted file mode 100644 index f85e531bc1..0000000000 --- a/.cursor/rules/api-patterns.mdc +++ /dev/null @@ -1,68 +0,0 @@ ---- -description: -globs: -alwaysApply: false ---- -# API Design Patterns - -Follow these established patterns for consistent API design and error handling. - -## Error Handling - -**Use Typed Errors Instead of String Matching** -- Use `errors.Is()` for error type checking instead of string comparisons -- Define custom error variables for domain-specific errors -- Error handling should be based on error types, not string content -- Someone may change error messaging and unintentionally affect status codes -- Follow established patterns for error type checking in the codebase - -**Return Consistent Error Responses** -- Always use `types.APIError` for structured error responses -- Use proper error constructor functions: `NewBadRequestError()`, `NewInternalServerError()`, `NewUnauthorizedError()`, etc. -- Let `jsonError()` helper automatically handle error type conversion -- Use `AppendFieldError()` for validation errors with specific field names - -## Handler Structure - -**Follow the Standard Handler Pattern** -- Use `bindJSON()` for request parsing with automatic error handling -- Use `json()` for successful responses with proper content-type headers -- Use `jsonError()` for error responses with consistent formatting -- Use `logError()` for structured logging with request context -- Return early on errors - don't continue processing after `bindJSON()` or other errors - -**Keep HTTP Layer Separate from Business Logic** -- Handlers should be thin - delegate complex logic to controllers -- Controllers handle business logic and return domain errors -- API layer converts domain errors to HTTP responses using typed errors -- Follow the controller pattern: handlers → controllers → managers → stores - -## Dependency Injection & Architecture - -**Use the Option Pattern for Constructors** -- All major components use functional options: `NewController(opts ...Option)` -- Options modify struct fields via closures: `func(c *Controller) { c.field = value }` -- Provide default implementations when nil dependencies are detected -- Use dependency injection to enable testing with mocks - -**Follow the Layered Architecture** -- **API Layer**: HTTP handlers, routing, middleware -- **Controller Layer**: Business logic orchestration, error handling -- **Manager Layer**: Domain-specific operations, workflow management -- **Store Layer**: Data persistence, in-memory or disk-based storage - -## Authentication & Authorization - -**Use Bearer Token Pattern** -- All protected routes use `authMiddleware` on subrouters -- Token extraction follows: `Authorization: Bearer ` -- Use typed errors for auth failures: `NewUnauthorizedError()` -- Authentication logic is centralized in auth controller - -## Route Organization - -**Group Related Routes Using Subrouters** -- Public routes go directly on main router -- Protected routes use `authenticatedRouter` subrouter with middleware -- Domain-specific routes use prefixed subrouters: `/install`, `/console` -- RESTful patterns: GET for retrieval, POST for actions/creation diff --git a/.cursor/rules/go-best-practices.mdc b/.cursor/rules/go-best-practices.mdc new file mode 100644 index 0000000000..c06ea97332 --- /dev/null +++ b/.cursor/rules/go-best-practices.mdc @@ -0,0 +1,223 @@ +--- +description: +globs: +alwaysApply: false +--- +# Go Best Practices + +Follow these best practices when writing Go code in this repository. This document is based on analysis of the existing codebase patterns. + +## Core Principles + +- **Clarity and Simplicity**: Write code that is easy to read, understand, and maintain. Favor explicit over implicit. +- **Dependency Injection**: Use dependency injection to decouple components and enable testing. The functional options pattern is the standard approach. +- **Interface-Driven Design**: Define behavior through interfaces to enable mocking and loose coupling. +- **Explicit Error Handling**: Handle all errors explicitly. Use structured error types for APIs. + +## Architecture Patterns + +### Functional Options Pattern + +Use the functional options pattern for component initialization. This is the standard across controllers and main components: + +```go +type ComponentOption func(*Component) + +func WithDependency(dep Dependency) ComponentOption { + return func(c *Component) { + c.dependency = dep + } +} + +func NewComponent(opts ...ComponentOption) (*Component, error) { + c := &Component{ + // Set defaults + } + + for _, opt := range opts { + opt(c) + } + + // Initialize defaults for nil dependencies + if c.dependency == nil { + c.dependency = NewDefaultDependency() + } + + return c, nil +} +``` + +### Interface Design + +- **Small, Focused Interfaces**: Keep interfaces small and focused on specific behavior +- **Interface Segregation**: Prefer multiple small interfaces over large ones +- **Testability**: All external dependencies should be behind interfaces for mocking +- **Naming**: Use descriptive names that indicate the behavior (e.g., `InstallationManager`, `NetUtils`) + +```go +type NetUtils interface { + ListValidNetworkInterfaces() ([]string, error) + DetermineBestNetworkInterface() (string, error) + FirstValidIPNet(networkInterface string) (*net.IPNet, error) +} +``` + +## Error Handling + +### Structured API Errors + +Use structured error types for APIs that need to return detailed error information: + +```go +type APIError struct { + StatusCode int `json:"status_code,omitempty"` + Message string `json:"message"` + Field string `json:"field,omitempty"` + Errors []*APIError `json:"errors,omitempty"` + err error `json:"-"` +} + +func (e *APIError) Error() string { /* implementation */ } +func (e *APIError) Unwrap() error { return e.err } +``` + +### Error Wrapping and Context + +- **Wrap Errors**: Always add context when propagating errors using `fmt.Errorf("operation failed: %w", err)` +- **Preserve Original**: Store the original error for unwrapping when using custom error types +- **Meaningful Messages**: Error messages should be actionable and include relevant context + +### Error Constructor Functions + +Create constructor functions for common error types: + +```go +func NewBadRequestError(err error) *APIError { + return &APIError{ + StatusCode: http.StatusBadRequest, + Message: err.Error(), + err: err, + } +} +``` + +## Naming Conventions + +### Package Names +- Use short, concise, all-lowercase names +- Avoid stuttering (don't repeat package name in exported types) +- Examples: `types`, `utils`, `install`, `auth` + +### Types and Functions +- **Exported Types**: Use `PascalCase` (e.g., `InstallController`, `APIError`) +- **Exported Functions**: Use `PascalCase` (e.g., `NewInstallController`, `ValidateConfig`) +- **Internal Functions**: Use `camelCase` (e.g., `bindJSON`, `logError`) +- **Variables**: Use `camelCase` for all variables + +### Interface Naming +- **Behavior-Based**: Name interfaces after what they do (e.g., `Controller`, `Manager`, `NetUtils`) +- **Avoid Generic Names**: Don't use generic suffixes like `Interface` unless necessary +- **Single Method**: For single-method interfaces, consider the "-er" suffix pattern + +## Concurrency and Thread Safety + +### Mutex Usage +- Use `sync.RWMutex` for read-heavy workloads +- Keep critical sections small +- Always use defer for unlocking: `defer mu.Unlock()` + +### Context Usage +- Pass `context.Context` as the first parameter to functions that may block or need cancellation +- Use `ctx context.Context` as the parameter name +- Don't store contexts in structs; pass them through function calls + +## Logging + +### Structured Logging with Logrus + +- Use `logrus.FieldLogger` interface for dependency injection +- Add contextual fields to log entries for better debugging +- Use appropriate log levels: `Debug`, `Info`, `Warn`, `Error` + +```go +logger.WithFields(logrus.Fields{ + "request_id": requestID, + "user_id": userID, +}).Info("processing request") +``` + +### Logging Patterns + +- **Error Logging**: Always log errors with context before returning them +- **Request Logging**: Log HTTP requests with relevant fields (method, path, status) +- **Discard Logger**: Use `logger.NewDiscardLogger()` for tests + +## JSON and HTTP Handling + +### JSON Tags and Serialization + +- Use appropriate JSON tags: `json:"field_name,omitempty"` +- Use `json:"-"` for fields that shouldn't be serialized +- Handle both marshaling and unmarshaling in your types + +### HTTP Handler Patterns + +- Use method receivers on the main API struct: `func (a *API) handlerName(...)` +- Use helper methods for common operations: `bindJSON`, `json`, `jsonError` +- Always validate input and handle errors appropriately + +## Testing Support + +### Testable Design + +- Use interfaces for all external dependencies +- Provide constructor functions that accept test doubles +- Create mock implementations alongside interfaces (e.g., `netutils_mock.go`) + +### Test Utilities + +- Provide `NewDiscardLogger()` for tests that need a logger +- Use dependency injection to swap real implementations with test doubles +- Create test-specific constructors when needed + +## Code Organization + +### Package Structure + +- **Controllers**: Business logic and orchestration (`controllers/`) +- **Types**: Data structures and domain models (`types/`) +- **Internal**: Implementation details not exposed to other packages (`internal/`) +- **Pkg**: Reusable utilities and helpers (`pkg/`) + +### File Organization + +- Keep related functionality together in the same file +- Use separate files for tests: `*_test.go` +- Use separate files for mocks: `*_mock.go` +- Keep files focused and reasonably sized (< 500 lines when possible) + +## Documentation + +### Code Comments + +- Document all exported types, functions, and methods +- Use complete sentences in comments +- Explain the "why" not just the "what" +- Include examples for complex APIs + +### Swagger/OpenAPI + +- Use Swagger annotations for HTTP handlers +- Document request/response structures +- Include error response documentation + +## Best Practices Summary + +- **Explicit Dependencies**: Use dependency injection with functional options +- **Interface Everything**: Put external dependencies behind interfaces +- **Handle All Errors**: Never ignore errors; wrap them with context +- **Structure for Testing**: Design code to be easily testable +- **Use Standard Libraries**: Prefer standard library over third-party when possible +- **Consistent Patterns**: Follow established patterns in the codebase +- **Clear Separation**: Separate concerns into appropriate packages and layers + diff --git a/.cursor/rules/go-ec-api-architecture.mdc b/.cursor/rules/go-ec-api-architecture.mdc new file mode 100644 index 0000000000..8de6e675dc --- /dev/null +++ b/.cursor/rules/go-ec-api-architecture.mdc @@ -0,0 +1,96 @@ +--- +description: +globs: +alwaysApply: false +--- +# Go Embedded Cluster API Architecture + +For comprehensive package structure and organization details, see [api/README.md](mdc:../api/README.md). + +## Handler → Controller → Manager Pattern + +Follow the three-layer architecture pattern consistently: + +### Handler Layer (`api/*.go`) +- **Purpose**: HTTP protocol handling only +- **Do**: Parse requests, call controllers, format responses, handle HTTP errors +- **Don't**: Business logic, external service calls, data persistence + +```go +func (a *API) postExample(w http.ResponseWriter, r *http.Request) { + var req types.ExampleRequest + if err := a.bindJSON(w, r, &req); err != nil { + return // bindJSON handles error response + } + + result, err := a.controller.DoSomething(r.Context(), req) + if err != nil { + a.logError(r, err, "failed to do something") + a.jsonError(w, r, err) + return + } + + a.json(w, r, http.StatusOK, result) +} +``` + +### Controller Layer (`api/controllers/*/`) +- **Purpose**: Business logic orchestration and workflow coordination +- **Do**: Coordinate managers, handle business validation, manage state transitions +- **Don't**: HTTP concerns, direct external service calls, low-level operations + +### Manager Layer (`api/internal/managers/*/`) +- **Purpose**: Domain-specific operations and external system integration +- **Do**: External system calls, data persistence, domain validation +- **Don't**: HTTP logic, cross-domain coordination + +## Essential Patterns + +### Error Handling +- Always log errors with context: `a.logError(r, err, "descriptive message")` +- Use structured errors: `types.NewBadRequestError(err)`, `types.NewInternalServerError(err)` +- Wrap errors with context: `fmt.Errorf("operation failed: %w", err)` + +### Request/Response +- Use `a.bindJSON(w, r, &struct)` for parsing JSON requests +- Use `a.json(w, r, statusCode, payload)` for success responses +- Use `a.jsonError(w, r, err)` for error responses + +### Dependencies +- Use functional options pattern for initialization +- Define interfaces for all external dependencies +- Inject dependencies via constructors + +### API Documentation +- Add Swagger annotations to all handlers: +```go +// @Summary Brief description +// @Description Detailed description +// @Tags category +// @Security bearerauth +// @Accept json +// @Produce json +// @Param request body types.RequestType true "Request description" +// @Success 200 {object} types.ResponseType +// @Router /path [method] +``` + +## Quick Reference + +### Adding New Functionality +- **New endpoint**: Add handler → create/update controller → define types +- **New business logic**: Add to appropriate controller or create new manager +- **New types**: Add to `api/types/` with proper JSON tags +- **New utilities**: Add to `api/pkg/` or `api/internal/` + +### File Naming +- Handlers: `api/{domain}.go` (e.g., `install.go`, `auth.go`) +- Controllers: `api/controllers/{domain}/controller.go` +- Managers: `api/internal/managers/{domain}/manager.go` +- Types: `api/types/{domain}.go` + +### Testing +- Unit tests: `*_test.go` alongside source files +- Integration tests: `api/integration/*_test.go` +- Use table-driven tests with `testify/assert` +- Mock all external dependencies diff --git a/.cursor/rules/go-testing.mdc b/.cursor/rules/go-testing.mdc index e19c73a9f1..0dd4692e18 100644 --- a/.cursor/rules/go-testing.mdc +++ b/.cursor/rules/go-testing.mdc @@ -3,119 +3,166 @@ description: globs: alwaysApply: false --- -# Go Testing Standards - -Follow these testing patterns for Go code to create valuable, maintainable tests. - -## Test Types and Strategy - -**Unit Tests** -- Test individual functions/methods in isolation -- Mock external dependencies -- Focus on business logic and edge cases -- Should be fast and not require external resources - -**Integration Tests** -- Test component interactions -- Use real implementations where feasible -- Place in appropriate directories (`integration/`, `*_integration_test.go`) -- May use external resources but should be self-contained - -## Test Value and Purpose - -**Focus on Behavioral Testing** -- Verify actual function behavior and business logic, not just object instantiation -- Test the contract and expected outcomes of functions -- Ensure tests validate meaningful functionality rather than trivial operations -- Include realistic scenarios that represent actual use cases - -**Comprehensive Test Coverage** -- Test both success and failure paths -- Include edge cases and boundary conditions -- Test error conditions explicitly with proper error type validation - -## Test Organization and Structure - -**Use Table-Driven Tests for Multiple Scenarios** -- Structure tests with `tests := []struct{}` pattern for comprehensive coverage -- Use descriptive test names that explain the scenario being tested -- Group related test cases logically within table-driven tests -- Follow the Arrange-Act-Assert pattern within each test case - -**Test Setup and Cleanup** -- Use `t.TempDir()` for temporary directories - it handles cleanup automatically -- Use `t.Cleanup()` for custom cleanup functions when needed -- Use `defer` statements for resource cleanup within test functions -- Prefer `t.Setenv()` over manual environment variable manipulation - -## Assertion and Error Testing - -**Use Testify Consistently** -- Use `require.NoError(t, err)` for critical assertions that should stop test execution -- Use `assert.NoError(t, err)` for non-critical assertions that allow test to continue -- Use `require.True(t, ok)` for type assertions and critical boolean checks -- Use `assert.Equal()`, `assert.Contains()`, `assert.Empty()` for value comparisons - -**Error Testing Best Practices** -- Test specific error types, not just that an error occurred -- Use `errors.Is()` and `errors.As()` for error checking in tests -- Test error messages when they're part of the API contract -- Test error propagation through the call stack - -## Mock Usage Guidelines - -**When to Use Mocks** -- Mock external dependencies, not the code under test -- Use interface-based mocking for better testability -- Consider using real implementations for lightweight dependencies -- Mock at the right level of abstraction - -**How to Use Mocks Effectively** -- Prefer dependency injection to enable clean mocking -- Set up mock expectations that match real-world scenarios using `mock.InOrder()` when sequence matters -- Don't mock simple value objects or data structures -- Always call `mockObject.AssertExpectations(t)` to verify mock usage - -## Specialized Testing Patterns - -### HTTP and API Testing - -**Use httptest for HTTP Testing** -- Use `httptest.NewServer()` for integration-style HTTP tests -- Use `httptest.NewRequest()` and `httptest.NewRecorder()` for handler unit tests -- Test both request handling and response formatting -- Verify HTTP status codes, headers, and response bodies - -**API Client Testing** -- Test both successful and error response scenarios -- Verify request construction (method, path, headers, body) -- Test error handling and proper error type conversion -- Use `defer server.Close()` to clean up test servers - -### Kubernetes and Controller Testing - -**Use envtest for Kubernetes Testing** -- Use `envtest.Environment` for testing Kubernetes controllers -- Use `fake.NewClientBuilder()` for lightweight client testing -- Set up proper schemes with `AddToScheme()` calls -- Use `t.Cleanup()` to stop test environments - -**Controller Runtime Testing Patterns** -- Create realistic Kubernetes objects for testing -- Test both creation and update scenarios -- Verify object state changes and status updates -- Use proper context handling with `logr.NewContext()` - -## Test Data and Resource Management - -**Keep Test Data Clean** -- Use test-specific data that doesn't depend on external state -- Create test data within test functions or table-driven test cases -- Use embedded test files when appropriate (`//go:embed`) -- Avoid shared mutable state between tests - -**Temporary Resources** -- Use `t.TempDir()` for file-based operations -- Clean up network resources (listeners, servers) with defer statements -- Use unique names/ports to avoid conflicts between parallel tests -- Create minimal test fixtures that focus on the specific test scenario \ No newline at end of file +# Testing + +## Testing Philosophy + +- **Test for behavior, not implementation**: Tests should verify the public behavior of a unit, not its internal implementation details. +- **Readable and maintainable**: Tests should be easy to read, understand, and maintain. A good test serves as documentation. +- **Fast and reliable**: Tests should run quickly and produce consistent results. + +## Test Organization + +### Test Types + +1. **Unit Tests**: Test individual functions/methods in isolation using mocks for dependencies +2. **Integration Tests**: Test the interaction between multiple components, often with real HTTP requests +3. **API Tests**: Test HTTP endpoints end-to-end with request/response validation + +### File Structure + +- Unit tests: `*_test.go` files alongside the code they test +- Integration tests: `api/integration/*_test.go` files +- Test assets: Store test data in `assets/` subdirectories within test packages + +## Test Structure Patterns + +### Table-Driven Tests + +Use table-driven tests for testing multiple scenarios. This is the standard pattern across the codebase: + +```go +func TestFunction(t *testing.T) { + tests := []struct { + name string + input InputType + setupMock func(*MockType) + expectedErr bool + expected OutputType + }{ + { + name: "success case", + // ... test case definition + }, + { + name: "error case", + // ... test case definition + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test implementation + }) + } +} +``` + +### Test Naming + +- Test functions: `TestFunctionName` or `TestTypeName_MethodName` +- Subtest names: Descriptive of the specific scenario being tested +- Use underscores for method tests: `TestAPIError_Error` + +## Tooling and Libraries + +### Core Libraries + +- **Standard testing**: Use the built-in `testing` package +- **Assertions**: Use `github.com/stretchr/testify/assert` and `github.com/stretchr/testify/require` + - `assert.*`: For non-fatal assertions that continue test execution + - `require.*`: For fatal assertions that stop test execution on failure +- **Mocks**: Use `github.com/stretchr/testify/mock` for creating mocks + +### HTTP Testing + +- **Unit-level HTTP testing**: Use `net/http/httptest` for testing individual handlers +- **Integration HTTP testing**: Create full API instances with `httptest.NewServer` +- **API Client testing**: Test both direct HTTP calls and API client wrapper methods + +## Testing Patterns by Component Type + +### API Handlers + +Test HTTP handlers by: +1. Creating mock dependencies +2. Setting up HTTP requests with `httptest.NewRequest` +3. Using `httptest.NewRecorder` to capture responses +4. Validating status codes, headers, and response bodies + +### Controllers (Business Logic) + +Test controllers by: +1. Mocking all dependencies (managers, utilities, etc.) +2. Setting up mock expectations with `mock.On()` and `mock.InOrder()` +3. Testing both success and error paths +4. Verifying mock expectations with `AssertExpectations(t)` + +### Types and Data Structures + +Test types by: +1. Validating serialization/deserialization (JSON marshaling) +2. Testing validation methods +3. Testing error handling and edge cases +4. Using table-driven tests for multiple validation scenarios + +### Integration Tests + +Structure integration tests by: +1. Creating real API instances with test configurations +2. Using `httptest.NewServer` for full HTTP testing +3. Testing authentication flows end-to-end +4. Validating complete request/response cycles +5. Testing with both direct HTTP calls and API client libraries + +## Mock Usage + +### Mock Setup Patterns + +- Use setup functions in table tests: `setupMock func(*MockType)` +- Use `mock.InOrder()` for sequential mock calls +- Use `mock.MatchedBy()` for complex argument matching +- Always call `AssertExpectations(t)` to verify all mocks were called + +### Mock Lifecycle + +- Create fresh mocks for each test case +- Set up mock expectations before running the test +- Verify mock expectations after the test completes + +## Test Utilities + +### Common Test Helpers + +- Use `logger.NewDiscardLogger()` for tests that need a logger +- Create test-specific environment setters for configuration +- Use temporary directories with `t.TempDir()` for file system tests +- Embed test assets using `//go:embed` for test data files + +### Context Usage + +- Use `t.Context()` for tests that need a context +- Pass contexts through to functions that expect them +- Don't create arbitrary timeouts unless testing timeout behavior + +## Error Testing + +### Error Validation Patterns + +- Test both error presence (`assert.Error(t, err)`) and absence (`assert.NoError(t, err)`) +- Validate error types when using custom errors: `apiErr, ok := err.(*types.APIError)` +- Test error messages and status codes for API errors +- Use `require.Error()` when the error is critical to continuing the test + +## Best Practices + +- **Isolation**: Each test should be independent and not rely on other tests +- **Cleanup**: Use `t.Cleanup()` or `defer` statements for resource cleanup +- **Deterministic**: Avoid time-based assertions unless testing time-sensitive behavior +- **Comprehensive**: Test both happy paths and error conditions +- **Mock verification**: Always verify that mocks were called as expected +- **Table tests**: Use table-driven tests for multiple similar scenarios +- **Descriptive names**: Make test names clearly describe what is being tested + + + diff --git a/operator/charts/embedded-cluster-operator/values.yaml b/operator/charts/embedded-cluster-operator/values.yaml index aceb2ec4fe..4e066a3194 100644 --- a/operator/charts/embedded-cluster-operator/values.yaml +++ b/operator/charts/embedded-cluster-operator/values.yaml @@ -13,6 +13,7 @@ image: pullPolicy: IfNotPresent utilsImage: busybox:latest +goldpingerImage: bloomberg/goldpinger:latest extraEnv: [] # - name: HTTP_PROXY @@ -56,6 +57,8 @@ affinity: operator: In values: - linux + - key: node-role.kubernetes.io/control-plane + operator: Exists metrics: enabled: false From df5b732aff5bc869ab151f42efa50f81833c158f Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Sun, 29 Jun 2025 11:05:41 -0400 Subject: [PATCH 03/11] more cursor rules --- .cursor/rules/clean-code.mdc | 30 +++ .cursor/rules/ec-config-types.mdc | 253 +++++++++++++++++++++++ design/installation-vs-runtime-config.md | 242 ++++++++++++++++++++++ 3 files changed, 525 insertions(+) create mode 100644 .cursor/rules/clean-code.mdc create mode 100644 .cursor/rules/ec-config-types.mdc create mode 100644 design/installation-vs-runtime-config.md diff --git a/.cursor/rules/clean-code.mdc b/.cursor/rules/clean-code.mdc new file mode 100644 index 0000000000..e3bb457de9 --- /dev/null +++ b/.cursor/rules/clean-code.mdc @@ -0,0 +1,30 @@ +--- +description: +globs: +alwaysApply: false +--- +# Clean Code + +Essential clean code principles for Go development in Embedded Cluster. + +## File Formatting + +### End of File +- **Always leave a single newline at the end of every file** +- This ensures proper POSIX compliance and clean git diffs + +## Comments + +### Keep Comments Concise +- **Comments should be brief and to the point** +- Explain *why*, not *what* the code does +- Avoid redundant comments that just repeat the code + +### Function Comments +- Use concise godoc format for exported functions +- Focus on purpose and important behavior +- Single line comments for simple functions + +### Inline Comments +- Use sparingly for complex logic +- Keep on same line when possible for short explanations diff --git a/.cursor/rules/ec-config-types.mdc b/.cursor/rules/ec-config-types.mdc new file mode 100644 index 0000000000..3d3aa08659 --- /dev/null +++ b/.cursor/rules/ec-config-types.mdc @@ -0,0 +1,253 @@ +--- +description: +globs: +alwaysApply: false +--- +# Installation Config vs Runtime Config + +Quick reference and actionable rules for working with Installation Config and Runtime Config in Embedded Cluster. + +> **📖 Comprehensive Documentation**: See `/design/installation-vs-runtime-config.md` for detailed architecture and understanding. + +## Quick Reference + +### When to Use Each Type + +| Aspect | Installation Config | Runtime Config | +|--------|-------------------|----------------| +| **Purpose** | User input and preferences | System state and environment | +| **Data Source** | User-provided values | Derived from installation config | +| **Persistence** | In-memory during installation | Disk-based (`/etc/embedded-cluster/ec.yaml`) | +| **Lifecycle** | Installation process only | Entire cluster lifetime | +| **Validation** | User-facing error messages | System constraint checking | +| **Responsibilities** | Input validation, user preferences | Environment variables, file paths | + +### Data Flow +``` +User Input → Installation Config → Runtime Config → System Environment +``` + +## Code Patterns + +### Adding User-Configurable Options + +#### 1. Add to Installation Config +```go +// Add field to types.InstallationConfig +type InstallationConfig struct { + // ... existing fields ... + NewUserOption string `json:"newUserOption"` +} + +// Add validation in installation manager +func (m *InstallationManager) ValidateConfig(config types.InstallationConfig) error { + if config.NewUserOption == "" { + return types.NewAPIError(types.ErrValidation, "newUserOption is required") + } + // Validate user-facing constraints + if !isValidUserOption(config.NewUserOption) { + return types.NewAPIError(types.ErrValidation, "invalid user option format") + } + return nil +} +``` + +#### 2. Update Runtime Config from Installation Config +```go +// Add system-level handling in runtime config +func (c *Controller) updateRuntimeFromInstallation(installConfig types.InstallationConfig) error { + // Transform user preference to system configuration + c.rc.SetNewSystemOption(installConfig.NewUserOption) + + // Apply to environment + if err := c.rc.SetEnv(); err != nil { + return fmt.Errorf("failed to set environment: %w", err) + } + + // Persist system state + if err := c.rc.WriteToDisk(); err != nil { + return fmt.Errorf("failed to persist runtime config: %w", err) + } + + return nil +} +``` + +### Configuration Update Flow + +#### Complete Update Pattern +```go +func (c *Controller) UpdateInstallationConfig(config types.InstallationConfig) error { + // Step 1: Validate user input (Installation Config responsibility) + if err := c.installationManager.ValidateConfig(config); err != nil { + return fmt.Errorf("invalid configuration: %w", err) + } + + // Step 2: Store installation config (temporary, in-memory) + if err := c.installationManager.SetConfig(config); err != nil { + return fmt.Errorf("failed to save config: %w", err) + } + + // Step 3: Transform to runtime config (system state) + c.rc.SetDataDir(config.DataDirectory) + c.rc.SetAdminConsolePort(config.AdminConsolePort) + c.rc.SetLocalArtifactMirrorPort(config.LocalArtifactMirrorPort) + + // Step 4: Apply environment changes (Runtime Config responsibility) + if err := c.rc.SetEnv(); err != nil { + return fmt.Errorf("failed to set environment: %w", err) + } + + // Step 5: Persist runtime state (permanent, disk-based) + if err := c.rc.WriteToDisk(); err != nil { + return fmt.Errorf("failed to persist runtime config: %w", err) + } + + return nil +} +``` + +### Validation Patterns + +#### Installation Config Validation (User-Facing) +```go +func (m *InstallationManager) ValidateConfig(config types.InstallationConfig) error { + // Port validation with user-friendly messages + if config.AdminConsolePort < 1024 { + return types.NewAPIError(types.ErrValidation, + "admin console port must be 1024 or higher") + } + + if isPortInUse(config.AdminConsolePort) { + return types.NewAPIError(types.ErrValidation, + fmt.Sprintf("port %d is already in use", config.AdminConsolePort)) + } + + // Network validation + if !isValidCIDR(config.GlobalCIDR) { + return types.NewAPIError(types.ErrValidation, + fmt.Sprintf("invalid network CIDR: %s", config.GlobalCIDR)) + } + + // Path validation + if config.DataDirectory == "" { + return types.NewAPIError(types.ErrValidation, + "data directory is required") + } + + return nil +} +``` + +#### Runtime Config Validation (System Constraints) +```go +func (rc *RuntimeConfig) SetDataDir(dir string) error { + // System-level validation + if dir == "" { + return fmt.Errorf("data directory cannot be empty") + } + + // Ensure directory can be created + if err := os.MkdirAll(dir, 0755); err != nil { + return fmt.Errorf("failed to create directory %s: %w", dir, err) + } + + // Check write permissions + if !isWritableDir(dir) { + return fmt.Errorf("directory not writable: %s", dir) + } + + rc.spec.DataDirectory = dir + return nil +} +``` + +## Testing Patterns + +### Installation Config Tests (User Input Focus) +```go +func TestInstallationConfigValidation(t *testing.T) { + tests := []struct { + name string + config types.InstallationConfig + wantErr bool + errMsg string + }{ + { + name: "valid config", + config: types.InstallationConfig{ + AdminConsolePort: 30000, + DataDirectory: "/tmp/test", + GlobalCIDR: "10.244.0.0/16", + }, + wantErr: false, + }, + { + name: "invalid port - too low", + config: types.InstallationConfig{ + AdminConsolePort: 80, + }, + wantErr: true, + errMsg: "port must be 1024 or higher", + }, + { + name: "invalid CIDR", + config: types.InstallationConfig{ + GlobalCIDR: "invalid-cidr", + }, + wantErr: true, + errMsg: "invalid network CIDR", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := manager.ValidateConfig(tt.config) + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errMsg) + } else { + require.NoError(t, err) + } + }) + } +} +``` + +### Runtime Config Tests (System State Focus) +```go +func TestRuntimeConfigPersistence(t *testing.T) { + tempDir := t.TempDir() + + // Create runtime config with system state + rc := runtimeconfig.New(ecv1beta1.RuntimeConfigSpec{ + DataDirectory: tempDir, + AdminConsolePort: 30000, + }) + + // Test persistence + err := rc.WriteToDisk() + require.NoError(t, err) + + // Test loading + loaded, err := runtimeconfig.NewFromDisk() + require.NoError(t, err) + + // Verify system state preserved + assert.Equal(t, tempDir, loaded.DataDirectory()) + assert.Equal(t, 30000, loaded.AdminConsolePort()) +} + +func TestRuntimeConfigEnvironment(t *testing.T) { + rc := runtimeconfig.New(ecv1beta1.RuntimeConfigSpec{ + AdminConsolePort: 30000, + }) + + // Test environment variable application + err := rc.SetEnv() + require.NoError(t, err) + + // Verify environment variables set + assert.Equal(t, "30000", os.Getenv("ADMIN_CONSOLE_PORT")) +} +``` diff --git a/design/installation-vs-runtime-config.md b/design/installation-vs-runtime-config.md new file mode 100644 index 0000000000..d110f9fd94 --- /dev/null +++ b/design/installation-vs-runtime-config.md @@ -0,0 +1,242 @@ +# Installation Config vs Runtime Config + +This document explains the key architectural differences between Installation Config and Runtime Config in the Embedded Cluster project. + +## Overview + +The project uses two primary configuration types that work in tandem but serve fundamentally different purposes: + +**Installation Config** → **Runtime Config** + +Understanding this distinction is crucial for proper configuration management in the system. + +## Core Differences + +### Installation Config (`api/types/installation.go`) +**Purpose**: User-configurable installation parameters +**Scope**: Installation-specific settings that affect cluster setup +**Lifecycle**: Created during installation, exists only during installation process +**Persistence**: Stored in memory via `installation.Store` interface during installation +**Owner**: User/Installation Process +**Nature**: Source of truth for user preferences + +### Runtime Config (`pkg/runtimeconfig/`) +**Purpose**: System runtime configuration and environment management +**Scope**: Process environment, file paths, and runtime behavior +**Lifecycle**: Lives for the entire cluster lifetime +**Persistence**: Persisted to disk at `/etc/embedded-cluster/ec.yaml` +**Owner**: System/Cluster Runtime +**Nature**: Derived state from installation preferences + +## Key Architectural Distinctions + +### 1. Source vs. Derived State + +**Installation Config**: +- Contains user-provided values and choices +- Represents "what the user wants" +- Input to the system +- Source of truth for user preferences + +**Runtime Config**: +- Contains computed values and system paths +- Represents "how the system is configured" +- System state representation +- Derived from user preferences + +### 2. Lifecycle and Persistence Strategy + +**Installation Config**: +- Created during installation process +- Lives in memory during installation +- Discarded after installation completes +- Temporary, process-specific storage + +**Runtime Config**: +- Created from installation config +- Persisted to disk for durability +- Lives for entire cluster lifetime +- Survives restarts and upgrades + +### 3. Data Flow Direction + +``` +User Input → Installation Config → Runtime Config → System Environment +``` + +The data flows in one direction only: +- User provides input to Installation Config +- Installation Config validates and stores user preferences +- Runtime Config receives validated preferences and computes system state +- System environment is configured from Runtime Config + +### 4. Validation Responsibilities + +**Installation Config**: +- User-facing validation with clear error messages +- Business rule validation (port conflicts, network ranges) +- Input format validation (required fields, valid formats) +- Cross-field validation to ensure configuration coherence + +**Runtime Config**: +- System constraint validation (path permissions, disk space) +- Environment validation (required directories exist) +- Internal consistency checks for derived values + +## Detailed Comparison + +### Network Configuration Handling + +**Installation Config**: +- Stores user-specified port preferences +- Handles network interface selection +- Manages CIDR range choices +- Validates port availability and network conflicts + +**Runtime Config**: +- Provides access to configured ports via getter methods +- Manages environment variables for network configuration +- Handles system-level network configuration +- Applies network settings to the running system + +### Directory and Path Management + +**Installation Config**: +- Stores user preference for high-level directory location +- Validates directory accessibility and permissions +- Represents user choice for data storage location + +**Runtime Config**: +- Computes all specific system paths based on user preference +- Manages file and directory locations for all cluster components +- Provides path accessors for kubeconfig, K0s config, etc. +- Handles path creation and management + +### Configuration Updates and Synchronization + +**Installation Config Flow**: +1. User provides configuration via API/UI +2. Configuration is validated for user-facing constraints +3. Configuration is stored in memory during installation +4. Configuration drives runtime config updates + +**Runtime Config Flow**: +1. Receives updates from validated installation config +2. Computes derived values and system paths +3. Updates environment variables +4. Persists configuration to disk for durability + +## Data Transformation Examples + +### Port Configuration +- **Installation Config**: User specifies `AdminConsolePort: 30000` +- **Runtime Config**: Sets `ADMIN_CONSOLE_PORT=30000` in environment and provides `AdminConsolePort()` accessor + +### Directory Configuration +- **Installation Config**: User specifies `DataDirectory: "/opt/my-cluster"` +- **Runtime Config**: Computes all system paths: + - `EmbeddedClusterHomeDirectory()` → `"/opt/my-cluster"` + - `PathToKubeConfig()` → `"/opt/my-cluster/k0s/pki/admin.conf"` + - `K0sConfigPath()` → `"/opt/my-cluster/k0s/k0s.yaml"` + +## Architectural Patterns + +### Separation of Concerns +- **Installation Config**: Handles user interface and input validation +- **Runtime Config**: Manages system state and environment +- Clear boundaries prevent mixing of responsibilities + +### Single Direction Data Flow +- Data flows from Installation Config to Runtime Config only +- No reverse dependencies or circular updates +- Clear transformation pipeline from user input to system state + +### Appropriate Persistence Strategy +- **Installation Config**: Temporary, in-memory during installation +- **Runtime Config**: Permanent, disk-based for cluster lifetime +- Each uses persistence mechanism appropriate to its lifecycle + +### Validation at Boundaries +- User input validated in Installation Config layer +- System constraints validated in Runtime Config layer +- No duplication of validation logic across layers + +### Error Handling Strategy +- **Installation Config**: User-friendly error messages with actionable guidance +- **Runtime Config**: System-level error messages for operational issues +- Structured error types for consistent error handling + +## Integration Architecture + +### Controller Integration Pattern +The typical flow in controllers follows this pattern: + +1. **Validate user input** using Installation Config validation +2. **Store installation config** temporarily in memory +3. **Transform to runtime config** by updating system state +4. **Apply environment changes** via Runtime Config +5. **Persist runtime state** to disk for durability + +This pattern ensures proper separation of concerns and maintains data flow integrity. + +### Configuration Synchronization +- Installation Config and Runtime Config are kept synchronized during installation +- Environment variables are updated after runtime config changes +- Configuration changes are persisted atomically +- Partial update failures are handled gracefully + +## Design Principles + +### Single Source of Truth +- Each configuration value has one authoritative source +- Configuration flows in one direction through the layers +- Duplication of configuration data is avoided + +### Fail Fast Philosophy +- Configuration is validated as early as possible +- Clear error messages are provided for configuration problems +- Invalid configuration is prevented from propagating through the system + +### Composability and Modularity +- Configuration types can be composed and combined +- Functional options pattern enables flexible configuration +- Partial configuration and defaults are supported + +### Testability +- Configuration logic is easily testable in isolation +- Dependencies can be mocked or stubbed +- Configuration state is observable and verifiable + +## Common Anti-Patterns to Avoid + +### Bypassing the Configuration Flow +- Don't put user input directly into Runtime Config +- Don't skip Installation Config validation +- Don't reverse the data flow direction + +### Mixing Responsibilities +- Don't put user-facing validation in Runtime Config +- Don't put system-level concerns in Installation Config +- Don't duplicate validation logic across layers + +### Ignoring Persistence Requirements +- Don't forget to persist Runtime Config changes +- Don't assume Installation Config will survive restarts +- Don't mix temporary and permanent storage strategies + +## Future Architectural Considerations + +### Configuration Evolution +- Plan for adding new fields to both config types +- Maintain backward compatibility for runtime config persistence +- Consider migration strategies for configuration schema changes + +### Dynamic Configuration Updates +- Some runtime config may support hot updates in the future +- Installation config changes may require cluster restart +- Design for partial configuration updates where appropriate + +### Multi-Node Configuration Distribution +- Consider how configuration is distributed in multi-node clusters +- Plan for configuration consistency across cluster nodes +- Design for configuration conflict resolution mechanisms From fcf3b7841ade4127d289a0b723872e96788dbb5e Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Wed, 2 Jul 2025 14:52:45 -0400 Subject: [PATCH 04/11] trim design doc --- design/installation-vs-runtime-config.md | 56 ------------------------ 1 file changed, 56 deletions(-) diff --git a/design/installation-vs-runtime-config.md b/design/installation-vs-runtime-config.md index d110f9fd94..63c8b7f4da 100644 --- a/design/installation-vs-runtime-config.md +++ b/design/installation-vs-runtime-config.md @@ -184,59 +184,3 @@ This pattern ensures proper separation of concerns and maintains data flow integ - Environment variables are updated after runtime config changes - Configuration changes are persisted atomically - Partial update failures are handled gracefully - -## Design Principles - -### Single Source of Truth -- Each configuration value has one authoritative source -- Configuration flows in one direction through the layers -- Duplication of configuration data is avoided - -### Fail Fast Philosophy -- Configuration is validated as early as possible -- Clear error messages are provided for configuration problems -- Invalid configuration is prevented from propagating through the system - -### Composability and Modularity -- Configuration types can be composed and combined -- Functional options pattern enables flexible configuration -- Partial configuration and defaults are supported - -### Testability -- Configuration logic is easily testable in isolation -- Dependencies can be mocked or stubbed -- Configuration state is observable and verifiable - -## Common Anti-Patterns to Avoid - -### Bypassing the Configuration Flow -- Don't put user input directly into Runtime Config -- Don't skip Installation Config validation -- Don't reverse the data flow direction - -### Mixing Responsibilities -- Don't put user-facing validation in Runtime Config -- Don't put system-level concerns in Installation Config -- Don't duplicate validation logic across layers - -### Ignoring Persistence Requirements -- Don't forget to persist Runtime Config changes -- Don't assume Installation Config will survive restarts -- Don't mix temporary and permanent storage strategies - -## Future Architectural Considerations - -### Configuration Evolution -- Plan for adding new fields to both config types -- Maintain backward compatibility for runtime config persistence -- Consider migration strategies for configuration schema changes - -### Dynamic Configuration Updates -- Some runtime config may support hot updates in the future -- Installation config changes may require cluster restart -- Design for partial configuration updates where appropriate - -### Multi-Node Configuration Distribution -- Consider how configuration is distributed in multi-node clusters -- Plan for configuration consistency across cluster nodes -- Design for configuration conflict resolution mechanisms From c76bb1db99142539b837817b98371feeb0d2f90a Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Wed, 2 Jul 2025 14:57:43 -0400 Subject: [PATCH 05/11] simplify --- .cursor/rules/ec-config-types.mdc | 256 +++--------------------------- 1 file changed, 22 insertions(+), 234 deletions(-) diff --git a/.cursor/rules/ec-config-types.mdc b/.cursor/rules/ec-config-types.mdc index 3d3aa08659..2de45ffd59 100644 --- a/.cursor/rules/ec-config-types.mdc +++ b/.cursor/rules/ec-config-types.mdc @@ -5,249 +5,37 @@ alwaysApply: false --- # Installation Config vs Runtime Config -Quick reference and actionable rules for working with Installation Config and Runtime Config in Embedded Cluster. +Simple actionable rules for working with configs in Embedded Cluster. -> **📖 Comprehensive Documentation**: See `/design/installation-vs-runtime-config.md` for detailed architecture and understanding. +> **📖 Architecture Details**: See `/design/installation-vs-runtime-config.md` for comprehensive design documentation. -## Quick Reference +## When to Use Each -### When to Use Each Type +- **Installation Config**: User input and preferences during installation +- **Runtime Config**: System state and environment variables during cluster lifetime -| Aspect | Installation Config | Runtime Config | -|--------|-------------------|----------------| -| **Purpose** | User input and preferences | System state and environment | -| **Data Source** | User-provided values | Derived from installation config | -| **Persistence** | In-memory during installation | Disk-based (`/etc/embedded-cluster/ec.yaml`) | -| **Lifecycle** | Installation process only | Entire cluster lifetime | -| **Validation** | User-facing error messages | System constraint checking | -| **Responsibilities** | Input validation, user preferences | Environment variables, file paths | - -### Data Flow +## Data Flow Rule ``` User Input → Installation Config → Runtime Config → System Environment ``` -## Code Patterns - -### Adding User-Configurable Options +## Actionable Rules -#### 1. Add to Installation Config -```go -// Add field to types.InstallationConfig -type InstallationConfig struct { - // ... existing fields ... - NewUserOption string `json:"newUserOption"` -} +### Adding New User Options +1. Add field to `types.InstallationConfig` +2. Add validation in installation manager with user-friendly error messages +3. Transform to runtime config in controller +4. Apply to environment via `SetEnv()` and persist via `WriteToDisk()` -// Add validation in installation manager -func (m *InstallationManager) ValidateConfig(config types.InstallationConfig) error { - if config.NewUserOption == "" { - return types.NewAPIError(types.ErrValidation, "newUserOption is required") - } - // Validate user-facing constraints - if !isValidUserOption(config.NewUserOption) { - return types.NewAPIError(types.ErrValidation, "invalid user option format") - } - return nil -} -``` - -#### 2. Update Runtime Config from Installation Config -```go -// Add system-level handling in runtime config -func (c *Controller) updateRuntimeFromInstallation(installConfig types.InstallationConfig) error { - // Transform user preference to system configuration - c.rc.SetNewSystemOption(installConfig.NewUserOption) - - // Apply to environment - if err := c.rc.SetEnv(); err != nil { - return fmt.Errorf("failed to set environment: %w", err) - } - - // Persist system state - if err := c.rc.WriteToDisk(); err != nil { - return fmt.Errorf("failed to persist runtime config: %w", err) - } - - return nil -} -``` +### Configuration Updates +Follow this order: +1. Validate user input (Installation Config) +2. Store installation config (in-memory) +3. Transform to runtime config (system state) +4. Apply environment changes +5. Persist to disk -### Configuration Update Flow +### Validation Guidelines +- **Installation Config**: User-friendly error messages using `types.NewAPIError()` +- **Runtime Config**: System constraint checking with standard errors -#### Complete Update Pattern -```go -func (c *Controller) UpdateInstallationConfig(config types.InstallationConfig) error { - // Step 1: Validate user input (Installation Config responsibility) - if err := c.installationManager.ValidateConfig(config); err != nil { - return fmt.Errorf("invalid configuration: %w", err) - } - - // Step 2: Store installation config (temporary, in-memory) - if err := c.installationManager.SetConfig(config); err != nil { - return fmt.Errorf("failed to save config: %w", err) - } - - // Step 3: Transform to runtime config (system state) - c.rc.SetDataDir(config.DataDirectory) - c.rc.SetAdminConsolePort(config.AdminConsolePort) - c.rc.SetLocalArtifactMirrorPort(config.LocalArtifactMirrorPort) - - // Step 4: Apply environment changes (Runtime Config responsibility) - if err := c.rc.SetEnv(); err != nil { - return fmt.Errorf("failed to set environment: %w", err) - } - - // Step 5: Persist runtime state (permanent, disk-based) - if err := c.rc.WriteToDisk(); err != nil { - return fmt.Errorf("failed to persist runtime config: %w", err) - } - - return nil -} -``` - -### Validation Patterns - -#### Installation Config Validation (User-Facing) -```go -func (m *InstallationManager) ValidateConfig(config types.InstallationConfig) error { - // Port validation with user-friendly messages - if config.AdminConsolePort < 1024 { - return types.NewAPIError(types.ErrValidation, - "admin console port must be 1024 or higher") - } - - if isPortInUse(config.AdminConsolePort) { - return types.NewAPIError(types.ErrValidation, - fmt.Sprintf("port %d is already in use", config.AdminConsolePort)) - } - - // Network validation - if !isValidCIDR(config.GlobalCIDR) { - return types.NewAPIError(types.ErrValidation, - fmt.Sprintf("invalid network CIDR: %s", config.GlobalCIDR)) - } - - // Path validation - if config.DataDirectory == "" { - return types.NewAPIError(types.ErrValidation, - "data directory is required") - } - - return nil -} -``` - -#### Runtime Config Validation (System Constraints) -```go -func (rc *RuntimeConfig) SetDataDir(dir string) error { - // System-level validation - if dir == "" { - return fmt.Errorf("data directory cannot be empty") - } - - // Ensure directory can be created - if err := os.MkdirAll(dir, 0755); err != nil { - return fmt.Errorf("failed to create directory %s: %w", dir, err) - } - - // Check write permissions - if !isWritableDir(dir) { - return fmt.Errorf("directory not writable: %s", dir) - } - - rc.spec.DataDirectory = dir - return nil -} -``` - -## Testing Patterns - -### Installation Config Tests (User Input Focus) -```go -func TestInstallationConfigValidation(t *testing.T) { - tests := []struct { - name string - config types.InstallationConfig - wantErr bool - errMsg string - }{ - { - name: "valid config", - config: types.InstallationConfig{ - AdminConsolePort: 30000, - DataDirectory: "/tmp/test", - GlobalCIDR: "10.244.0.0/16", - }, - wantErr: false, - }, - { - name: "invalid port - too low", - config: types.InstallationConfig{ - AdminConsolePort: 80, - }, - wantErr: true, - errMsg: "port must be 1024 or higher", - }, - { - name: "invalid CIDR", - config: types.InstallationConfig{ - GlobalCIDR: "invalid-cidr", - }, - wantErr: true, - errMsg: "invalid network CIDR", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := manager.ValidateConfig(tt.config) - if tt.wantErr { - require.Error(t, err) - assert.Contains(t, err.Error(), tt.errMsg) - } else { - require.NoError(t, err) - } - }) - } -} -``` - -### Runtime Config Tests (System State Focus) -```go -func TestRuntimeConfigPersistence(t *testing.T) { - tempDir := t.TempDir() - - // Create runtime config with system state - rc := runtimeconfig.New(ecv1beta1.RuntimeConfigSpec{ - DataDirectory: tempDir, - AdminConsolePort: 30000, - }) - - // Test persistence - err := rc.WriteToDisk() - require.NoError(t, err) - - // Test loading - loaded, err := runtimeconfig.NewFromDisk() - require.NoError(t, err) - - // Verify system state preserved - assert.Equal(t, tempDir, loaded.DataDirectory()) - assert.Equal(t, 30000, loaded.AdminConsolePort()) -} - -func TestRuntimeConfigEnvironment(t *testing.T) { - rc := runtimeconfig.New(ecv1beta1.RuntimeConfigSpec{ - AdminConsolePort: 30000, - }) - - // Test environment variable application - err := rc.SetEnv() - require.NoError(t, err) - - // Verify environment variables set - assert.Equal(t, "30000", os.Getenv("ADMIN_CONSOLE_PORT")) -} -``` From 22fd59c9a307e5c25a526c1fb2d723fe938279af Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Wed, 2 Jul 2025 16:40:43 -0400 Subject: [PATCH 06/11] update rules --- .cursor/rules/ec-config-types.mdc | 41 --------- .cursor/rules/go-best-practices.mdc | 110 +---------------------- .cursor/rules/go-ec-api-architecture.mdc | 96 ++++++++++++-------- .cursor/rules/go-testing.mdc | 64 ------------- api/README.md | 37 ++++++++ design/installation-vs-runtime-config.md | 86 ------------------ 6 files changed, 101 insertions(+), 333 deletions(-) delete mode 100644 .cursor/rules/ec-config-types.mdc diff --git a/.cursor/rules/ec-config-types.mdc b/.cursor/rules/ec-config-types.mdc deleted file mode 100644 index 2de45ffd59..0000000000 --- a/.cursor/rules/ec-config-types.mdc +++ /dev/null @@ -1,41 +0,0 @@ ---- -description: -globs: -alwaysApply: false ---- -# Installation Config vs Runtime Config - -Simple actionable rules for working with configs in Embedded Cluster. - -> **📖 Architecture Details**: See `/design/installation-vs-runtime-config.md` for comprehensive design documentation. - -## When to Use Each - -- **Installation Config**: User input and preferences during installation -- **Runtime Config**: System state and environment variables during cluster lifetime - -## Data Flow Rule -``` -User Input → Installation Config → Runtime Config → System Environment -``` - -## Actionable Rules - -### Adding New User Options -1. Add field to `types.InstallationConfig` -2. Add validation in installation manager with user-friendly error messages -3. Transform to runtime config in controller -4. Apply to environment via `SetEnv()` and persist via `WriteToDisk()` - -### Configuration Updates -Follow this order: -1. Validate user input (Installation Config) -2. Store installation config (in-memory) -3. Transform to runtime config (system state) -4. Apply environment changes -5. Persist to disk - -### Validation Guidelines -- **Installation Config**: User-friendly error messages using `types.NewAPIError()` -- **Runtime Config**: System constraint checking with standard errors - diff --git a/.cursor/rules/go-best-practices.mdc b/.cursor/rules/go-best-practices.mdc index c06ea97332..80b0f0b90f 100644 --- a/.cursor/rules/go-best-practices.mdc +++ b/.cursor/rules/go-best-practices.mdc @@ -5,14 +5,14 @@ alwaysApply: false --- # Go Best Practices -Follow these best practices when writing Go code in this repository. This document is based on analysis of the existing codebase patterns. +Follow these best practices when writing Go code in this repository. ## Core Principles - **Clarity and Simplicity**: Write code that is easy to read, understand, and maintain. Favor explicit over implicit. - **Dependency Injection**: Use dependency injection to decouple components and enable testing. The functional options pattern is the standard approach. - **Interface-Driven Design**: Define behavior through interfaces to enable mocking and loose coupling. -- **Explicit Error Handling**: Handle all errors explicitly. Use structured error types for APIs. +- **Explicit Error Handling**: Handle all errors explicitly. Use structured error types when appropriate. ## Architecture Patterns @@ -64,43 +64,12 @@ type NetUtils interface { ## Error Handling -### Structured API Errors - -Use structured error types for APIs that need to return detailed error information: - -```go -type APIError struct { - StatusCode int `json:"status_code,omitempty"` - Message string `json:"message"` - Field string `json:"field,omitempty"` - Errors []*APIError `json:"errors,omitempty"` - err error `json:"-"` -} - -func (e *APIError) Error() string { /* implementation */ } -func (e *APIError) Unwrap() error { return e.err } -``` - ### Error Wrapping and Context - **Wrap Errors**: Always add context when propagating errors using `fmt.Errorf("operation failed: %w", err)` - **Preserve Original**: Store the original error for unwrapping when using custom error types - **Meaningful Messages**: Error messages should be actionable and include relevant context -### Error Constructor Functions - -Create constructor functions for common error types: - -```go -func NewBadRequestError(err error) *APIError { - return &APIError{ - StatusCode: http.StatusBadRequest, - Message: err.Error(), - err: err, - } -} -``` - ## Naming Conventions ### Package Names @@ -109,9 +78,9 @@ func NewBadRequestError(err error) *APIError { - Examples: `types`, `utils`, `install`, `auth` ### Types and Functions -- **Exported Types**: Use `PascalCase` (e.g., `InstallController`, `APIError`) +- **Exported Types**: Use `PascalCase` (e.g., `InstallController`, `NetUtils`) - **Exported Functions**: Use `PascalCase` (e.g., `NewInstallController`, `ValidateConfig`) -- **Internal Functions**: Use `camelCase` (e.g., `bindJSON`, `logError`) +- **Internal Functions**: Use `camelCase` (e.g., `processRequest`, `validateInput`) - **Variables**: Use `camelCase` for all variables ### Interface Naming @@ -149,75 +118,4 @@ logger.WithFields(logrus.Fields{ ### Logging Patterns - **Error Logging**: Always log errors with context before returning them -- **Request Logging**: Log HTTP requests with relevant fields (method, path, status) - **Discard Logger**: Use `logger.NewDiscardLogger()` for tests - -## JSON and HTTP Handling - -### JSON Tags and Serialization - -- Use appropriate JSON tags: `json:"field_name,omitempty"` -- Use `json:"-"` for fields that shouldn't be serialized -- Handle both marshaling and unmarshaling in your types - -### HTTP Handler Patterns - -- Use method receivers on the main API struct: `func (a *API) handlerName(...)` -- Use helper methods for common operations: `bindJSON`, `json`, `jsonError` -- Always validate input and handle errors appropriately - -## Testing Support - -### Testable Design - -- Use interfaces for all external dependencies -- Provide constructor functions that accept test doubles -- Create mock implementations alongside interfaces (e.g., `netutils_mock.go`) - -### Test Utilities - -- Provide `NewDiscardLogger()` for tests that need a logger -- Use dependency injection to swap real implementations with test doubles -- Create test-specific constructors when needed - -## Code Organization - -### Package Structure - -- **Controllers**: Business logic and orchestration (`controllers/`) -- **Types**: Data structures and domain models (`types/`) -- **Internal**: Implementation details not exposed to other packages (`internal/`) -- **Pkg**: Reusable utilities and helpers (`pkg/`) - -### File Organization - -- Keep related functionality together in the same file -- Use separate files for tests: `*_test.go` -- Use separate files for mocks: `*_mock.go` -- Keep files focused and reasonably sized (< 500 lines when possible) - -## Documentation - -### Code Comments - -- Document all exported types, functions, and methods -- Use complete sentences in comments -- Explain the "why" not just the "what" -- Include examples for complex APIs - -### Swagger/OpenAPI - -- Use Swagger annotations for HTTP handlers -- Document request/response structures -- Include error response documentation - -## Best Practices Summary - -- **Explicit Dependencies**: Use dependency injection with functional options -- **Interface Everything**: Put external dependencies behind interfaces -- **Handle All Errors**: Never ignore errors; wrap them with context -- **Structure for Testing**: Design code to be easily testable -- **Use Standard Libraries**: Prefer standard library over third-party when possible -- **Consistent Patterns**: Follow established patterns in the codebase -- **Clear Separation**: Separate concerns into appropriate packages and layers - diff --git a/.cursor/rules/go-ec-api-architecture.mdc b/.cursor/rules/go-ec-api-architecture.mdc index 8de6e675dc..8cfa4023e3 100644 --- a/.cursor/rules/go-ec-api-architecture.mdc +++ b/.cursor/rules/go-ec-api-architecture.mdc @@ -3,66 +3,80 @@ description: globs: alwaysApply: false --- -# Go Embedded Cluster API Architecture +# Go Embedded Cluster API Implementation Guidelines -For comprehensive package structure and organization details, see [api/README.md](mdc:../api/README.md). +For comprehensive architecture and package structure details, see [api/README.md](mdc:../api/README.md). -## Handler → Controller → Manager Pattern +## Essential Implementation Patterns -Follow the three-layer architecture pattern consistently: +### Error Handling -### Handler Layer (`api/*.go`) -- **Purpose**: HTTP protocol handling only -- **Do**: Parse requests, call controllers, format responses, handle HTTP errors -- **Don't**: Business logic, external service calls, data persistence +#### Structured API Errors -```go -func (a *API) postExample(w http.ResponseWriter, r *http.Request) { - var req types.ExampleRequest - if err := a.bindJSON(w, r, &req); err != nil { - return // bindJSON handles error response - } +Use structured error types for APIs that need to return detailed error information: - result, err := a.controller.DoSomething(r.Context(), req) - if err != nil { - a.logError(r, err, "failed to do something") - a.jsonError(w, r, err) - return - } - - a.json(w, r, http.StatusOK, result) +```go +type APIError struct { + StatusCode int `json:"status_code,omitempty"` + Message string `json:"message"` + Field string `json:"field,omitempty"` + Errors []*APIError `json:"errors,omitempty"` + err error `json:"-"` } + +func (e *APIError) Error() string { /* implementation */ } +func (e *APIError) Unwrap() error { return e.err } ``` -### Controller Layer (`api/controllers/*/`) -- **Purpose**: Business logic orchestration and workflow coordination -- **Do**: Coordinate managers, handle business validation, manage state transitions -- **Don't**: HTTP concerns, direct external service calls, low-level operations +#### Error Constructor Functions -### Manager Layer (`api/internal/managers/*/`) -- **Purpose**: Domain-specific operations and external system integration -- **Do**: External system calls, data persistence, domain validation -- **Don't**: HTTP logic, cross-domain coordination +Create constructor functions for common API error types: -## Essential Patterns +```go +func NewBadRequestError(err error) *APIError { + return &APIError{ + StatusCode: http.StatusBadRequest, + Message: err.Error(), + err: err, + } +} +``` + +#### API Error Handling Patterns -### Error Handling - Always log errors with context: `a.logError(r, err, "descriptive message")` - Use structured errors: `types.NewBadRequestError(err)`, `types.NewInternalServerError(err)` - Wrap errors with context: `fmt.Errorf("operation failed: %w", err)` +### HTTP Handler Patterns + +- Use method receivers on the main API struct: `func (a *API) handlerName(...)` +- Use helper methods for common operations: `bindJSON`, `json`, `jsonError` +- Always validate input and handle errors appropriately +- Log HTTP requests with relevant fields (method, path, status) + ### Request/Response + - Use `a.bindJSON(w, r, &struct)` for parsing JSON requests - Use `a.json(w, r, statusCode, payload)` for success responses - Use `a.jsonError(w, r, err)` for error responses +### JSON Tags and Serialization + +- Use appropriate JSON tags: `json:"field_name,omitempty"` +- Use `json:"-"` for fields that shouldn't be serialized +- Handle both marshaling and unmarshaling in your types + ### Dependencies + - Use functional options pattern for initialization - Define interfaces for all external dependencies - Inject dependencies via constructors ### API Documentation -- Add Swagger annotations to all handlers: + +Add Swagger annotations to all handlers: + ```go // @Summary Brief description // @Description Detailed description @@ -75,21 +89,31 @@ func (a *API) postExample(w http.ResponseWriter, r *http.Request) { // @Router /path [method] ``` -## Quick Reference +#### Swagger/OpenAPI Requirements + +- Use Swagger annotations for HTTP handlers +- Document request/response structures +- Include error response documentation +- Document authentication requirements + +## Implementation Quick Reference ### Adding New Functionality + - **New endpoint**: Add handler → create/update controller → define types - **New business logic**: Add to appropriate controller or create new manager - **New types**: Add to `api/types/` with proper JSON tags - **New utilities**: Add to `api/pkg/` or `api/internal/` -### File Naming +### File Naming Conventions + - Handlers: `api/{domain}.go` (e.g., `install.go`, `auth.go`) - Controllers: `api/controllers/{domain}/controller.go` - Managers: `api/internal/managers/{domain}/manager.go` - Types: `api/types/{domain}.go` -### Testing +### Testing Requirements + - Unit tests: `*_test.go` alongside source files - Integration tests: `api/integration/*_test.go` - Use table-driven tests with `testify/assert` diff --git a/.cursor/rules/go-testing.mdc b/.cursor/rules/go-testing.mdc index 0dd4692e18..d95f673e89 100644 --- a/.cursor/rules/go-testing.mdc +++ b/.cursor/rules/go-testing.mdc @@ -31,33 +31,6 @@ alwaysApply: false Use table-driven tests for testing multiple scenarios. This is the standard pattern across the codebase: -```go -func TestFunction(t *testing.T) { - tests := []struct { - name string - input InputType - setupMock func(*MockType) - expectedErr bool - expected OutputType - }{ - { - name: "success case", - // ... test case definition - }, - { - name: "error case", - // ... test case definition - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Test implementation - }) - } -} -``` - ### Test Naming - Test functions: `TestFunctionName` or `TestTypeName_MethodName` @@ -129,40 +102,3 @@ Structure integration tests by: - Create fresh mocks for each test case - Set up mock expectations before running the test - Verify mock expectations after the test completes - -## Test Utilities - -### Common Test Helpers - -- Use `logger.NewDiscardLogger()` for tests that need a logger -- Create test-specific environment setters for configuration -- Use temporary directories with `t.TempDir()` for file system tests -- Embed test assets using `//go:embed` for test data files - -### Context Usage - -- Use `t.Context()` for tests that need a context -- Pass contexts through to functions that expect them -- Don't create arbitrary timeouts unless testing timeout behavior - -## Error Testing - -### Error Validation Patterns - -- Test both error presence (`assert.Error(t, err)`) and absence (`assert.NoError(t, err)`) -- Validate error types when using custom errors: `apiErr, ok := err.(*types.APIError)` -- Test error messages and status codes for API errors -- Use `require.Error()` when the error is critical to continuing the test - -## Best Practices - -- **Isolation**: Each test should be independent and not rely on other tests -- **Cleanup**: Use `t.Cleanup()` or `defer` statements for resource cleanup -- **Deterministic**: Avoid time-based assertions unless testing time-sensitive behavior -- **Comprehensive**: Test both happy paths and error conditions -- **Mock verification**: Always verify that mocks were called as expected -- **Table tests**: Use table-driven tests for multiple similar scenarios -- **Descriptive names**: Make test names clearly describe what is being tested - - - diff --git a/api/README.md b/api/README.md index 83dcc6223a..612d2d36ba 100644 --- a/api/README.md +++ b/api/README.md @@ -66,6 +66,43 @@ Provides a client library for interacting with the API. The client package imple ## Best Practices +### Architecture Pattern: Handler → Controller → Manager + +Follow the three-layer architecture pattern consistently: + +#### Handler Layer (`api/*.go`) +- **Purpose**: HTTP protocol handling only +- **Do**: Parse requests, call controllers, format responses, handle HTTP errors +- **Don't**: Business logic, external service calls, data persistence + +```go +func (a *API) postExample(w http.ResponseWriter, r *http.Request) { + var req types.ExampleRequest + if err := a.bindJSON(w, r, &req); err != nil { + return // bindJSON handles error response + } + + result, err := a.controller.DoSomething(r.Context(), req) + if err != nil { + a.logError(r, err, "failed to do something") + a.jsonError(w, r, err) + return + } + + a.json(w, r, http.StatusOK, result) +} +``` + +#### Controller Layer (`api/controllers/*/`) +- **Purpose**: Business logic orchestration and workflow coordination +- **Do**: Coordinate managers, handle business validation, manage state transitions +- **Don't**: HTTP concerns, direct external service calls, low-level operations + +#### Manager Layer (`api/internal/managers/*/`) +- **Purpose**: Domain-specific operations and external system integration +- **Do**: External system calls, data persistence, domain validation +- **Don't**: HTTP logic, cross-domain coordination + 1. **Error Handling**: - Use custom error types from `/types` - Include proper error wrapping and context diff --git a/design/installation-vs-runtime-config.md b/design/installation-vs-runtime-config.md index 63c8b7f4da..40dfa11cf9 100644 --- a/design/installation-vs-runtime-config.md +++ b/design/installation-vs-runtime-config.md @@ -98,89 +98,3 @@ The data flows in one direction only: - Manages environment variables for network configuration - Handles system-level network configuration - Applies network settings to the running system - -### Directory and Path Management - -**Installation Config**: -- Stores user preference for high-level directory location -- Validates directory accessibility and permissions -- Represents user choice for data storage location - -**Runtime Config**: -- Computes all specific system paths based on user preference -- Manages file and directory locations for all cluster components -- Provides path accessors for kubeconfig, K0s config, etc. -- Handles path creation and management - -### Configuration Updates and Synchronization - -**Installation Config Flow**: -1. User provides configuration via API/UI -2. Configuration is validated for user-facing constraints -3. Configuration is stored in memory during installation -4. Configuration drives runtime config updates - -**Runtime Config Flow**: -1. Receives updates from validated installation config -2. Computes derived values and system paths -3. Updates environment variables -4. Persists configuration to disk for durability - -## Data Transformation Examples - -### Port Configuration -- **Installation Config**: User specifies `AdminConsolePort: 30000` -- **Runtime Config**: Sets `ADMIN_CONSOLE_PORT=30000` in environment and provides `AdminConsolePort()` accessor - -### Directory Configuration -- **Installation Config**: User specifies `DataDirectory: "/opt/my-cluster"` -- **Runtime Config**: Computes all system paths: - - `EmbeddedClusterHomeDirectory()` → `"/opt/my-cluster"` - - `PathToKubeConfig()` → `"/opt/my-cluster/k0s/pki/admin.conf"` - - `K0sConfigPath()` → `"/opt/my-cluster/k0s/k0s.yaml"` - -## Architectural Patterns - -### Separation of Concerns -- **Installation Config**: Handles user interface and input validation -- **Runtime Config**: Manages system state and environment -- Clear boundaries prevent mixing of responsibilities - -### Single Direction Data Flow -- Data flows from Installation Config to Runtime Config only -- No reverse dependencies or circular updates -- Clear transformation pipeline from user input to system state - -### Appropriate Persistence Strategy -- **Installation Config**: Temporary, in-memory during installation -- **Runtime Config**: Permanent, disk-based for cluster lifetime -- Each uses persistence mechanism appropriate to its lifecycle - -### Validation at Boundaries -- User input validated in Installation Config layer -- System constraints validated in Runtime Config layer -- No duplication of validation logic across layers - -### Error Handling Strategy -- **Installation Config**: User-friendly error messages with actionable guidance -- **Runtime Config**: System-level error messages for operational issues -- Structured error types for consistent error handling - -## Integration Architecture - -### Controller Integration Pattern -The typical flow in controllers follows this pattern: - -1. **Validate user input** using Installation Config validation -2. **Store installation config** temporarily in memory -3. **Transform to runtime config** by updating system state -4. **Apply environment changes** via Runtime Config -5. **Persist runtime state** to disk for durability - -This pattern ensures proper separation of concerns and maintains data flow integrity. - -### Configuration Synchronization -- Installation Config and Runtime Config are kept synchronized during installation -- Environment variables are updated after runtime config changes -- Configuration changes are persisted atomically -- Partial update failures are handled gracefully From 703490641594f74395df3deebbc4abc88c4b0f4d Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Wed, 2 Jul 2025 18:58:13 -0400 Subject: [PATCH 07/11] update rules --- .cursor/rules/clean-code.mdc | 16 +++- .cursor/rules/frontend-rules.mdc | 28 ++++++- .cursor/rules/go-best-practices.mdc | 94 +++++++++++++----------- .cursor/rules/go-ec-api-architecture.mdc | 47 +++++------- .cursor/rules/go-testing.mdc | 68 ++++++++++++++++- 5 files changed, 178 insertions(+), 75 deletions(-) diff --git a/.cursor/rules/clean-code.mdc b/.cursor/rules/clean-code.mdc index e3bb457de9..6ccd9a8e63 100644 --- a/.cursor/rules/clean-code.mdc +++ b/.cursor/rules/clean-code.mdc @@ -1,6 +1,6 @@ --- description: -globs: +globs: *.go alwaysApply: false --- # Clean Code @@ -20,6 +20,20 @@ Essential clean code principles for Go development in Embedded Cluster. - Explain *why*, not *what* the code does - Avoid redundant comments that just repeat the code +### Comment Quality + +- **Write self-explanatory comments that clearly explain the purpose and context**: Explain WHY, not just WHAT the code does + ```go + // Good - explains WHY and provides context + // Retry 3 times because API can be temporarily unavailable during deployment + for i := 0; i < 3; i++ { + if err := checkAPIHealth(); err == nil { + break + } + time.Sleep(time.Second * 2) + } + ``` + ### Function Comments - Use concise godoc format for exported functions - Focus on purpose and important behavior diff --git a/.cursor/rules/frontend-rules.mdc b/.cursor/rules/frontend-rules.mdc index 48a8afea6b..bf3482a85c 100644 --- a/.cursor/rules/frontend-rules.mdc +++ b/.cursor/rules/frontend-rules.mdc @@ -1,6 +1,6 @@ --- description: -globs: +globs: web/*.tsx,web/*.ts,web/*.js,web/*.jsx alwaysApply: false --- # manager experience (front end) rules and guidelines @@ -43,3 +43,29 @@ alwaysApply: false Please validate and run unit tests with `npm run test:unit ` from the `web` directory. + +## React Architecture Patterns + +### Context Usage Guidelines + +- **Only use React Context when you have props drilling**: Prefer props for simple parent-child communication to avoid unnecessary complexity + ```jsx + // Good - simple props for parent-child + + + // Bad - unnecessary Context for simple cases + + + + ``` + +### Component Visibility Control + +- **Control component visibility at parent level, not within the component itself**: This creates cleaner conditional rendering patterns + ```jsx + // Good - parent controls visibility + {showModal && setShowModal(false)} />} + + // Bad - component controls its own visibility + + ``` diff --git a/.cursor/rules/go-best-practices.mdc b/.cursor/rules/go-best-practices.mdc index 80b0f0b90f..d2d4a99fdf 100644 --- a/.cursor/rules/go-best-practices.mdc +++ b/.cursor/rules/go-best-practices.mdc @@ -1,6 +1,6 @@ --- description: -globs: +globs: *.go alwaysApply: false --- # Go Best Practices @@ -18,34 +18,7 @@ Follow these best practices when writing Go code in this repository. ### Functional Options Pattern -Use the functional options pattern for component initialization. This is the standard across controllers and main components: - -```go -type ComponentOption func(*Component) - -func WithDependency(dep Dependency) ComponentOption { - return func(c *Component) { - c.dependency = dep - } -} - -func NewComponent(opts ...ComponentOption) (*Component, error) { - c := &Component{ - // Set defaults - } - - for _, opt := range opts { - opt(c) - } - - // Initialize defaults for nil dependencies - if c.dependency == nil { - c.dependency = NewDefaultDependency() - } - - return c, nil -} -``` +Use the functional options pattern for component initialization. This is the standard across controllers and main components. ### Interface Design @@ -54,21 +27,31 @@ func NewComponent(opts ...ComponentOption) (*Component, error) { - **Testability**: All external dependencies should be behind interfaces for mocking - **Naming**: Use descriptive names that indicate the behavior (e.g., `InstallationManager`, `NetUtils`) -```go -type NetUtils interface { - ListValidNetworkInterfaces() ([]string, error) - DetermineBestNetworkInterface() (string, error) - FirstValidIPNet(networkInterface string) (*net.IPNet, error) -} -``` - ## Error Handling ### Error Wrapping and Context - **Wrap Errors**: Always add context when propagating errors using `fmt.Errorf("operation failed: %w", err)` +- **Use %w verb for error wrapping**: Use `%w` instead of `%v` when wrapping errors to maintain the error chain + ```go + return fmt.Errorf("failed to process: %w", err) // Good + return fmt.Errorf("failed to process: %v", err) // Bad + ``` - **Preserve Original**: Store the original error for unwrapping when using custom error types - **Meaningful Messages**: Error messages should be actionable and include relevant context +- **Use type-safe error handling**: Check error types instead of string matching to avoid brittle code + ```go + if errors.Is(err, context.DeadlineExceeded) { ... } // Good + if strings.Contains(err.Error(), "deadline") { ... } // Bad + ``` + +### Error Message Consistency + +- **Go error strings should start with lowercase letter and not end with punctuation**: Follow Go conventions + ```go + return errors.New("failed to connect") // Good + return errors.New("Failed to connect.") // Bad + ``` ## Naming Conventions @@ -88,6 +71,36 @@ type NetUtils interface { - **Avoid Generic Names**: Don't use generic suffixes like `Interface` unless necessary - **Single Method**: For single-method interfaces, consider the "-er" suffix pattern +### Variable Naming Clarity + +- **Use distinct variable names for file paths vs configuration objects**: Distinguish between file locations, raw data, and parsed objects + ```go + configPath := "/etc/config.yaml" // file location + configBytes := []byte{} // raw data + config := &Config{} // parsed object + ``` + +## Configuration Management + +### Input Validation and Defaults + +- **Check before setting defaults**: Always verify if user-provided fields are empty before setting defaults to avoid overriding user input + ```go + if config.DataDirectory == "" { + config.DataDirectory = "/opt/default" + } + ``` + +## CLI Design + +### Flag Naming and Help + +- **Ensure all CLI flags appear in help menu with consistent naming patterns**: Use kebab-case for multi-word flags + ```go + installCmd.Flags().String("admin-console-namespace", "", "Namespace for admin console") // Good + installCmd.Flags().String("adminconsolenamespace", "", "Namespace for admin console") // Bad + ``` + ## Concurrency and Thread Safety ### Mutex Usage @@ -108,13 +121,6 @@ type NetUtils interface { - Add contextual fields to log entries for better debugging - Use appropriate log levels: `Debug`, `Info`, `Warn`, `Error` -```go -logger.WithFields(logrus.Fields{ - "request_id": requestID, - "user_id": userID, -}).Info("processing request") -``` - ### Logging Patterns - **Error Logging**: Always log errors with context before returning them diff --git a/.cursor/rules/go-ec-api-architecture.mdc b/.cursor/rules/go-ec-api-architecture.mdc index 8cfa4023e3..8e4336f5c7 100644 --- a/.cursor/rules/go-ec-api-architecture.mdc +++ b/.cursor/rules/go-ec-api-architecture.mdc @@ -1,6 +1,6 @@ --- description: -globs: +globs: api/*.go alwaysApply: false --- # Go Embedded Cluster API Implementation Guidelines @@ -23,24 +23,11 @@ type APIError struct { Errors []*APIError `json:"errors,omitempty"` err error `json:"-"` } - -func (e *APIError) Error() string { /* implementation */ } -func (e *APIError) Unwrap() error { return e.err } ``` #### Error Constructor Functions -Create constructor functions for common API error types: - -```go -func NewBadRequestError(err error) *APIError { - return &APIError{ - StatusCode: http.StatusBadRequest, - Message: err.Error(), - err: err, - } -} -``` +Create constructor functions for common API error types. #### API Error Handling Patterns @@ -48,6 +35,21 @@ func NewBadRequestError(err error) *APIError { - Use structured errors: `types.NewBadRequestError(err)`, `types.NewInternalServerError(err)` - Wrap errors with context: `fmt.Errorf("operation failed: %w", err)` +#### Specific API Error Types + +- **Use specific API error types**: Always use `NewBadRequestError`, `NewInternalServerError`, etc. instead of generic errors for proper HTTP status codes + ```go + return NewBadRequestError(errors.New("invalid input")) // Good + return errors.New("invalid input") // Bad + ``` + +- **Return consistent HTTP status codes**: Use appropriate APIError types for different error conditions + ```go + return NewBadRequestError(errors.New("invalid input")) // 400 + return NewForbiddenError(errors.New("access denied")) // 403 + return NewInternalServerError(errors.New("database failed")) // 500 + ``` + ### HTTP Handler Patterns - Use method receivers on the main API struct: `func (a *API) handlerName(...)` @@ -75,19 +77,7 @@ func NewBadRequestError(err error) *APIError { ### API Documentation -Add Swagger annotations to all handlers: - -```go -// @Summary Brief description -// @Description Detailed description -// @Tags category -// @Security bearerauth -// @Accept json -// @Produce json -// @Param request body types.RequestType true "Request description" -// @Success 200 {object} types.ResponseType -// @Router /path [method] -``` +Add Swagger annotations to all handlers for API documentation. #### Swagger/OpenAPI Requirements @@ -118,3 +108,4 @@ Add Swagger annotations to all handlers: - Integration tests: `api/integration/*_test.go` - Use table-driven tests with `testify/assert` - Mock all external dependencies + diff --git a/.cursor/rules/go-testing.mdc b/.cursor/rules/go-testing.mdc index d95f673e89..73b0ddedbc 100644 --- a/.cursor/rules/go-testing.mdc +++ b/.cursor/rules/go-testing.mdc @@ -1,6 +1,6 @@ --- description: -globs: +globs: *.go alwaysApply: false --- # Testing @@ -102,3 +102,69 @@ Structure integration tests by: - Create fresh mocks for each test case - Set up mock expectations before running the test - Verify mock expectations after the test completes + +## Test Structure + +### Test Naming +- Use descriptive test names that clearly indicate what is being tested +- Follow the pattern: `TestFunctionName_Scenario_ExpectedResult` +- Example: `TestValidateConfig_EmptyDataDirectory_ReturnsError` + +### Use Semantic Naming for Test Cases + +- **Name test cases descriptively**: Use meaningful names for test cases rather than generic names like "test case 1" + ```go + tests := []struct { + name string + input *Config + expectedError string + }{ + { + name: "empty data directory should return error", + input: &Config{DataDirectory: ""}, + expectedError: "data directory cannot be empty", + }, + { + name: "test case 2", + input: &Config{DataDirectory: "/opt/data"}, + expectedError: "", + }, + } + ``` + +### Test Organization +- Group related tests in the same file +- Use subtests for variations of the same test case +- Keep test files focused on testing a single component or feature + +### Test Coverage and Quality + +- **Remove tests that don't call any functions or validate actual behavior**: Tests should assert on real functionality, not just test data structures + ```go + func TestValidateConfig_EmptyDirectory_ReturnsError(t *testing.T) { + config := &Config{DataDirectory: ""} + err := ValidateConfig(config) // Actually calls the function + assert.Error(t, err) + } + + func TestConfigStruct_Fields_Exist(t *testing.T) { + config := &Config{} + assert.NotNil(t, config) // Just tests data structure + } + ``` + +### Mock Management + +- **Extend existing mock interfaces instead of creating new ones**: Reduce maintenance burden by adding methods to existing mocks + ```go + // Good - extend existing mock + type mockInstallController struct { + GetStatusFunc func() Status + } + + // Bad - create separate mock + type mockStatusController struct { + GetStatusFunc func() Status + } + ``` + From e903b222cfb6e87baad176c93b4d61beb32140c7 Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Wed, 2 Jul 2025 18:59:45 -0400 Subject: [PATCH 08/11] revert changes to api readme --- api/README.md | 37 ------------------------------------- 1 file changed, 37 deletions(-) diff --git a/api/README.md b/api/README.md index 612d2d36ba..83dcc6223a 100644 --- a/api/README.md +++ b/api/README.md @@ -66,43 +66,6 @@ Provides a client library for interacting with the API. The client package imple ## Best Practices -### Architecture Pattern: Handler → Controller → Manager - -Follow the three-layer architecture pattern consistently: - -#### Handler Layer (`api/*.go`) -- **Purpose**: HTTP protocol handling only -- **Do**: Parse requests, call controllers, format responses, handle HTTP errors -- **Don't**: Business logic, external service calls, data persistence - -```go -func (a *API) postExample(w http.ResponseWriter, r *http.Request) { - var req types.ExampleRequest - if err := a.bindJSON(w, r, &req); err != nil { - return // bindJSON handles error response - } - - result, err := a.controller.DoSomething(r.Context(), req) - if err != nil { - a.logError(r, err, "failed to do something") - a.jsonError(w, r, err) - return - } - - a.json(w, r, http.StatusOK, result) -} -``` - -#### Controller Layer (`api/controllers/*/`) -- **Purpose**: Business logic orchestration and workflow coordination -- **Do**: Coordinate managers, handle business validation, manage state transitions -- **Don't**: HTTP concerns, direct external service calls, low-level operations - -#### Manager Layer (`api/internal/managers/*/`) -- **Purpose**: Domain-specific operations and external system integration -- **Do**: External system calls, data persistence, domain validation -- **Don't**: HTTP logic, cross-domain coordination - 1. **Error Handling**: - Use custom error types from `/types` - Include proper error wrapping and context From fe6dd668a9b54f551a4b4515b1c0e8314d208334 Mon Sep 17 00:00:00 2001 From: Diamon Wiggins <38189728+diamonwiggins@users.noreply.github.com> Date: Mon, 7 Jul 2025 10:07:18 -0400 Subject: [PATCH 09/11] Update .cursor/rules/go-ec-api-architecture.mdc Co-authored-by: Ethan Mosbaugh --- .cursor/rules/go-ec-api-architecture.mdc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cursor/rules/go-ec-api-architecture.mdc b/.cursor/rules/go-ec-api-architecture.mdc index 8e4336f5c7..dbaa2882f9 100644 --- a/.cursor/rules/go-ec-api-architecture.mdc +++ b/.cursor/rules/go-ec-api-architecture.mdc @@ -93,7 +93,7 @@ Add Swagger annotations to all handlers for API documentation. - **New endpoint**: Add handler → create/update controller → define types - **New business logic**: Add to appropriate controller or create new manager - **New types**: Add to `api/types/` with proper JSON tags -- **New utilities**: Add to `api/pkg/` or `api/internal/` +- **New utilities**: Add to `api/internal/` ### File Naming Conventions From 80f48a88ba376a6274ef5191127392a01788de5f Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Mon, 7 Jul 2025 10:30:23 -0400 Subject: [PATCH 10/11] update cursor rules --- .cursor/rules/go-best-practices.mdc | 6 +- .cursor/rules/go-ec-api-architecture.mdc | 8 +- .cursor/rules/go-testing.mdc | 64 +++++++++++---- design/installation-vs-runtime-config.md | 100 ----------------------- 4 files changed, 56 insertions(+), 122 deletions(-) delete mode 100644 design/installation-vs-runtime-config.md diff --git a/.cursor/rules/go-best-practices.mdc b/.cursor/rules/go-best-practices.mdc index d2d4a99fdf..b5bbb6b8b6 100644 --- a/.cursor/rules/go-best-practices.mdc +++ b/.cursor/rules/go-best-practices.mdc @@ -34,11 +34,11 @@ Use the functional options pattern for component initialization. This is the sta - **Wrap Errors**: Always add context when propagating errors using `fmt.Errorf("operation failed: %w", err)` - **Use %w verb for error wrapping**: Use `%w` instead of `%v` when wrapping errors to maintain the error chain ```go - return fmt.Errorf("failed to process: %w", err) // Good - return fmt.Errorf("failed to process: %v", err) // Bad + return fmt.Errorf("processing config: %w", err) // Good - contextual, no prefix + return fmt.Errorf("reading config file: %w", err) // Good - specific context ``` - **Preserve Original**: Store the original error for unwrapping when using custom error types -- **Meaningful Messages**: Error messages should be actionable and include relevant context +- **Meaningful Messages**: Error messages should be actionable and include relevant context without redundant prefixes - **Use type-safe error handling**: Check error types instead of string matching to avoid brittle code ```go if errors.Is(err, context.DeadlineExceeded) { ... } // Good diff --git a/.cursor/rules/go-ec-api-architecture.mdc b/.cursor/rules/go-ec-api-architecture.mdc index 8e4336f5c7..0bad80bec6 100644 --- a/.cursor/rules/go-ec-api-architecture.mdc +++ b/.cursor/rules/go-ec-api-architecture.mdc @@ -52,10 +52,10 @@ Create constructor functions for common API error types. ### HTTP Handler Patterns -- Use method receivers on the main API struct: `func (a *API) handlerName(...)` -- Use helper methods for common operations: `bindJSON`, `json`, `jsonError` +- Use dedicated Handler structs with HTTP method prefixed names: `func (h *Handler) PostConfigureInstallation(w http.ResponseWriter, r *http.Request) {}` +- Include HTTP method in handler names: `Get*`, `Post*`, `Put*`, `Delete*` +- Use helper methods for common operations: `utils.BindJSON`, `utils.JSON`, `utils.JSONError` - Always validate input and handle errors appropriately -- Log HTTP requests with relevant fields (method, path, status) ### Request/Response @@ -93,7 +93,7 @@ Add Swagger annotations to all handlers for API documentation. - **New endpoint**: Add handler → create/update controller → define types - **New business logic**: Add to appropriate controller or create new manager - **New types**: Add to `api/types/` with proper JSON tags -- **New utilities**: Add to `api/pkg/` or `api/internal/` +- **New utilities**: Add to `api/internal/` ### File Naming Conventions diff --git a/.cursor/rules/go-testing.mdc b/.cursor/rules/go-testing.mdc index 73b0ddedbc..fd19a0719f 100644 --- a/.cursor/rules/go-testing.mdc +++ b/.cursor/rules/go-testing.mdc @@ -34,7 +34,8 @@ Use table-driven tests for testing multiple scenarios. This is the standard patt ### Test Naming - Test functions: `TestFunctionName` or `TestTypeName_MethodName` -- Subtest names: Descriptive of the specific scenario being tested +- Use table-driven tests for multiple scenarios: `tests := []struct{name string, ...}{}` +- Subtest names: Descriptive of the specific scenario being tested in table entries - Use underscores for method tests: `TestAPIError_Error` ## Tooling and Libraries @@ -106,9 +107,36 @@ Structure integration tests by: ## Test Structure ### Test Naming -- Use descriptive test names that clearly indicate what is being tested -- Follow the pattern: `TestFunctionName_Scenario_ExpectedResult` -- Example: `TestValidateConfig_EmptyDataDirectory_ReturnsError` +- Use table-driven tests for multiple scenarios with descriptive test case names +- Test functions follow pattern: `TestFunctionName` for simple cases, `TestTypeName_MethodName` for methods +- Test case names in tables should be descriptive: `name: "empty data directory should return error"` +- Example of proper table-driven test structure: + ```go + func TestValidateConfig(t *testing.T) { + tests := []struct { + name string + input *Config + expectedError string + }{ + { + name: "empty data directory should return error", + input: &Config{DataDirectory: ""}, + expectedError: "data directory cannot be empty", + }, + { + name: "valid directory should succeed", + input: &Config{DataDirectory: "/opt/data"}, + expectedError: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateConfig(tt.input) + // test logic here + }) + } + } + ``` ### Use Semantic Naming for Test Cases @@ -133,9 +161,9 @@ Structure integration tests by: ``` ### Test Organization -- Group related tests in the same file -- Use subtests for variations of the same test case -- Keep test files focused on testing a single component or feature +- Create one test file per source file (`file.go` → `file_test.go`) +- Use subtests for variations within table-driven tests +- Keep test files focused on testing their corresponding source file ### Test Coverage and Quality @@ -155,16 +183,22 @@ Structure integration tests by: ### Mock Management -- **Extend existing mock interfaces instead of creating new ones**: Reduce maintenance burden by adding methods to existing mocks +- **Reuse existing mock interfaces across tests**: Use the same mock type for the same interface rather than creating multiple mock types ```go - // Good - extend existing mock - type mockInstallController struct { - GetStatusFunc func() Status + // Good - reuse the same mock across different tests + type MockInstallationManager struct { + mock.Mock } + func (m *MockInstallationManager) GetConfig() (Config, error) { ... } + func (m *MockInstallationManager) SetConfig(config Config) error { ... } - // Bad - create separate mock - type mockStatusController struct { - GetStatusFunc func() Status - } + // Use this mock in multiple test files for the same interface + + // Bad - creating separate mocks for the same interface + type MockInstallManager struct { ... } // for installation tests + type MockConfigManager struct { ... } // for config tests + // Both implementing the same InstallationManager interface ``` +- **Follow consistent mock patterns**: All mocks use `testify/mock` with `type MockTypeName struct { mock.Mock }` +- **Implement complete interfaces**: Mock all methods of the interface even if some tests don't use them diff --git a/design/installation-vs-runtime-config.md b/design/installation-vs-runtime-config.md deleted file mode 100644 index 40dfa11cf9..0000000000 --- a/design/installation-vs-runtime-config.md +++ /dev/null @@ -1,100 +0,0 @@ -# Installation Config vs Runtime Config - -This document explains the key architectural differences between Installation Config and Runtime Config in the Embedded Cluster project. - -## Overview - -The project uses two primary configuration types that work in tandem but serve fundamentally different purposes: - -**Installation Config** → **Runtime Config** - -Understanding this distinction is crucial for proper configuration management in the system. - -## Core Differences - -### Installation Config (`api/types/installation.go`) -**Purpose**: User-configurable installation parameters -**Scope**: Installation-specific settings that affect cluster setup -**Lifecycle**: Created during installation, exists only during installation process -**Persistence**: Stored in memory via `installation.Store` interface during installation -**Owner**: User/Installation Process -**Nature**: Source of truth for user preferences - -### Runtime Config (`pkg/runtimeconfig/`) -**Purpose**: System runtime configuration and environment management -**Scope**: Process environment, file paths, and runtime behavior -**Lifecycle**: Lives for the entire cluster lifetime -**Persistence**: Persisted to disk at `/etc/embedded-cluster/ec.yaml` -**Owner**: System/Cluster Runtime -**Nature**: Derived state from installation preferences - -## Key Architectural Distinctions - -### 1. Source vs. Derived State - -**Installation Config**: -- Contains user-provided values and choices -- Represents "what the user wants" -- Input to the system -- Source of truth for user preferences - -**Runtime Config**: -- Contains computed values and system paths -- Represents "how the system is configured" -- System state representation -- Derived from user preferences - -### 2. Lifecycle and Persistence Strategy - -**Installation Config**: -- Created during installation process -- Lives in memory during installation -- Discarded after installation completes -- Temporary, process-specific storage - -**Runtime Config**: -- Created from installation config -- Persisted to disk for durability -- Lives for entire cluster lifetime -- Survives restarts and upgrades - -### 3. Data Flow Direction - -``` -User Input → Installation Config → Runtime Config → System Environment -``` - -The data flows in one direction only: -- User provides input to Installation Config -- Installation Config validates and stores user preferences -- Runtime Config receives validated preferences and computes system state -- System environment is configured from Runtime Config - -### 4. Validation Responsibilities - -**Installation Config**: -- User-facing validation with clear error messages -- Business rule validation (port conflicts, network ranges) -- Input format validation (required fields, valid formats) -- Cross-field validation to ensure configuration coherence - -**Runtime Config**: -- System constraint validation (path permissions, disk space) -- Environment validation (required directories exist) -- Internal consistency checks for derived values - -## Detailed Comparison - -### Network Configuration Handling - -**Installation Config**: -- Stores user-specified port preferences -- Handles network interface selection -- Manages CIDR range choices -- Validates port availability and network conflicts - -**Runtime Config**: -- Provides access to configured ports via getter methods -- Manages environment variables for network configuration -- Handles system-level network configuration -- Applies network settings to the running system From 132f460b153e934fd7c0f496a31f2e1ba588eb30 Mon Sep 17 00:00:00 2001 From: Diamon Wiggins Date: Mon, 7 Jul 2025 10:36:09 -0400 Subject: [PATCH 11/11] update logging patterns --- .cursor/rules/go-best-practices.mdc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cursor/rules/go-best-practices.mdc b/.cursor/rules/go-best-practices.mdc index b5bbb6b8b6..c076a17ca2 100644 --- a/.cursor/rules/go-best-practices.mdc +++ b/.cursor/rules/go-best-practices.mdc @@ -123,5 +123,5 @@ Use the functional options pattern for component initialization. This is the sta ### Logging Patterns -- **Error Logging**: Always log errors with context before returning them +- **Error Logging**: Always log errors at the outermost caller (bottom of the stack). All the context from the trace should be included in the message wrapping. - **Discard Logger**: Use `logger.NewDiscardLogger()` for tests