Skip to content

Commit f0d9f28

Browse files
diamonwigginssgalsaleh
diamonwiggins
and
sgalsaleh
authored
feat: pass config values from memory store to kots cli (#2440)
* Add API endpoint to set config values * f * f * pass config values from store to kots install * address conflicts * removing comments * fix dependency injection order and config values file generation * use same yaml library in tests * cleanup * update api docs and cursor rules * refactor * refactor * use appconfig store from controller to update config values * cleanup * cleanup * update cursor rules * fix lint * Update api/README.md Co-authored-by: Salah Al Saleh <sg.alsaleh@gmail.com> * pass config values to infra manager install function * remove config values file from disk after installation * f * f * cleanup * address feedback * Update api/controllers/linux/install/infra.go Co-authored-by: Salah Al Saleh <sg.alsaleh@gmail.com> --------- Co-authored-by: Salah Aldeen Al Saleh <sg.alsaleh@gmail.com>
1 parent b01ad9f commit f0d9f28

File tree

15 files changed

+805
-231
lines changed

15 files changed

+805
-231
lines changed

‎.cursor/rules/go-best-practices.mdc

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,20 @@ Use the functional options pattern for component initialization. This is the sta
3131

3232
### Error Wrapping and Context
3333

34-
- **Wrap Errors**: Always add context when propagating errors using `fmt.Errorf("operation failed: %w", err)`
34+
- **Wrap Errors**: Always add context when propagating errors using `fmt.Errorf("operation context: %w", err)`
3535
- **Use %w verb for error wrapping**: Use `%w` instead of `%v` when wrapping errors to maintain the error chain
36+
- **Avoid verbose prefixes**: Don't use "failed to" or "unable to" prefixes as they create repetitive error chains
3637
```go
37-
return fmt.Errorf("processing config: %w", err) // Good - contextual, no prefix
38-
return fmt.Errorf("reading config file: %w", err) // Good - specific context
38+
return fmt.Errorf("processing config: %w", err) // Good - concise context
39+
return fmt.Errorf("reading config file: %w", err) // Good - specific context
40+
return fmt.Errorf("failed to process config: %w", err) // Bad - verbose prefix
41+
return fmt.Errorf("unable to read config file: %w", err) // Bad - verbose prefix
42+
```
43+
- **Use gerunds or nouns for context**: Describe the operation being performed
44+
```go
45+
return fmt.Errorf("creating directory: %w", err) // Good - gerund
46+
return fmt.Errorf("config validation: %w", err) // Good - noun
47+
return fmt.Errorf("installing component: %w", err) // Good - gerund
3948
```
4049
- **Preserve Original**: Store the original error for unwrapping when using custom error types
4150
- **Meaningful Messages**: Error messages should be actionable and include relevant context without redundant prefixes
@@ -109,9 +118,9 @@ Use the functional options pattern for component initialization. This is the sta
109118
- Always use defer for unlocking: `defer mu.Unlock()`
110119

111120
### Context Usage
112-
- Pass `context.Context` as the first parameter to functions that may block or need cancellation
113-
- Use `ctx context.Context` as the parameter name
114-
- Don't store contexts in structs; pass them through function calls
121+
- **Context parameter placement**: When added, use `ctx context.Context` as the first parameter
122+
- **Only add context when it's actually used**: Don't add `context.Context` parameters unless the function will actually use it for cancellation, deadlines, or passing values
123+
- **Don't store contexts in structs**: Pass them through function calls
115124

116125
## Logging
117126

‎.cursor/rules/go-ec-api-architecture.mdc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,22 @@ Create constructor functions for common API error types.
7575
- Define interfaces for all external dependencies
7676
- Inject dependencies via constructors
7777

78+
### Manager Architecture (API-Specific)
79+
80+
- **Never inject managers into other managers**: Pass data/config between managers via controller
81+
- **Controller orchestration**: Controllers read from one manager and pass data to another
82+
- **Semantic option naming**: Use `WithConfigFile()` for paths, `WithConfigData()` for actual data
83+
- **Independent testing**: Mock managers separately, test controller orchestration logic independently
84+
85+
```go
86+
// ✅ CORRECT: Pass data through controller
87+
configData, err := controller.managerA.GetData()
88+
controller.managerB = NewManagerB(WithConfigData(configData))
89+
90+
// ❌ INCORRECT: Manager-to-manager injection
91+
controller.managerB = NewManagerB(WithManagerA(controller.managerA))
92+
```
93+
7894
### API Documentation
7995

8096
Add Swagger annotations to all handlers for API documentation.

‎.cursor/rules/go-testing.mdc

Lines changed: 112 additions & 169 deletions
Original file line numberDiff line numberDiff line change
@@ -5,200 +5,143 @@ alwaysApply: false
55
---
66
# Testing
77

8-
## Testing Philosophy
9-
10-
- **Test for behavior, not implementation**: Tests should verify the public behavior of a unit, not its internal implementation details.
11-
- **Readable and maintainable**: Tests should be easy to read, understand, and maintain. A good test serves as documentation.
12-
- **Fast and reliable**: Tests should run quickly and produce consistent results.
13-
14-
## Test Organization
8+
## Core Rules
9+
10+
1. **One test function per code function** - Never create multiple test functions for the same code function
11+
2. **Use table-driven tests** for multiple scenarios within a single test function
12+
3. **Test behavior, not implementation** - Focus on public API behavior
13+
4. **Mock all external dependencies** using `testify/mock`
14+
15+
## Test Function Organization
16+
17+
### DON'T - Multiple Test Functions
18+
```go
19+
func TestNewController_BasicScenario(t *testing.T) { ... }
20+
func TestNewController_ErrorHandling(t *testing.T) { ... }
21+
func TestNewController_ConfigValues(t *testing.T) { ... }
22+
```
23+
24+
### DO - Single Function with Table Tests
25+
```go
26+
func TestNewController(t *testing.T) {
27+
tests := []struct {
28+
name string
29+
setup func()
30+
wantErr bool
31+
}{
32+
{name: "basic scenario", setup: ..., wantErr: false},
33+
{name: "error handling", setup: ..., wantErr: true},
34+
{name: "config values", setup: ..., wantErr: false},
35+
}
36+
for _, tt := range tests {
37+
t.Run(tt.name, func(t *testing.T) {
38+
// test implementation
39+
})
40+
}
41+
}
42+
```
43+
44+
## Test Types & Organization
1545

1646
### Test Types
17-
1847
1. **Unit Tests**: Test individual functions/methods in isolation using mocks for dependencies
19-
2. **Integration Tests**: Test the interaction between multiple components, often with real HTTP requests
48+
2. **Integration Tests**: Test interaction between multiple components, often with real HTTP requests
2049
3. **API Tests**: Test HTTP endpoints end-to-end with request/response validation
2150

2251
### File Structure
23-
2452
- Unit tests: `*_test.go` files alongside the code they test
2553
- Integration tests: `api/integration/*_test.go` files
2654
- Test assets: Store test data in `assets/` subdirectories within test packages
2755

28-
## Test Structure Patterns
29-
30-
### Table-Driven Tests
31-
32-
Use table-driven tests for testing multiple scenarios. This is the standard pattern across the codebase:
33-
34-
### Test Naming
56+
## Naming Conventions
57+
58+
- **Test functions**: `TestFunctionName` or `TestType_MethodName`
59+
- **Test cases**: Descriptive scenario names: `"empty input should return error"`
60+
- **Mock types**: `MockTypeName struct { mock.Mock }`
61+
- **Use semantic naming**: Meaningful test case names, not "test case 1"
62+
63+
## Required Libraries
64+
65+
```go
66+
import (
67+
"testing"
68+
"github.com/stretchr/testify/assert"
69+
"github.com/stretchr/testify/require"
70+
"github.com/stretchr/testify/mock"
71+
)
72+
```
73+
74+
## Mock Patterns
75+
76+
### Setup in Table Tests
77+
```go
78+
tests := []struct {
79+
name string
80+
setupMock func(*MockType)
81+
}{
82+
{
83+
name: "successful operation",
84+
setupMock: func(m *MockType) {
85+
m.On("Method", mock.Anything).Return(nil)
86+
},
87+
},
88+
}
89+
```
3590

36-
- Test functions: `TestFunctionName` or `TestTypeName_MethodName`
37-
- Use table-driven tests for multiple scenarios: `tests := []struct{name string, ...}{}`
38-
- Subtest names: Descriptive of the specific scenario being tested in table entries
39-
- Use underscores for method tests: `TestAPIError_Error`
40-
41-
## Tooling and Libraries
42-
43-
### Core Libraries
44-
45-
- **Standard testing**: Use the built-in `testing` package
46-
- **Assertions**: Use `github.com/stretchr/testify/assert` and `github.com/stretchr/testify/require`
47-
- `assert.*`: For non-fatal assertions that continue test execution
48-
- `require.*`: For fatal assertions that stop test execution on failure
49-
- **Mocks**: Use `github.com/stretchr/testify/mock` for creating mocks
50-
51-
### HTTP Testing
91+
### Mock Lifecycle
92+
- Create fresh mocks for each test case
93+
- Use `mock.InOrder()` for sequential mock calls
94+
- Use `mock.MatchedBy()` for complex argument matching
95+
- **Always verify mocks**: `mockManager.AssertExpectations(t)`
5296

53-
- **Unit-level HTTP testing**: Use `net/http/httptest` for testing individual handlers
54-
- **Integration HTTP testing**: Create full API instances with `httptest.NewServer`
55-
- **API Client testing**: Test both direct HTTP calls and API client wrapper methods
97+
### Mock Management
98+
- **Reuse existing mock interfaces** across tests - use same mock type for same interface
99+
- **Implement complete interfaces** - mock all methods even if some tests don't use them
100+
- **Follow consistent patterns** - all mocks use `testify/mock` with `type MockTypeName struct { mock.Mock }`
56101

57-
## Testing Patterns by Component Type
102+
## Testing Patterns by Component
58103

59104
### API Handlers
60-
61-
Test HTTP handlers by:
62-
1. Creating mock dependencies
63-
2. Setting up HTTP requests with `httptest.NewRequest`
64-
3. Using `httptest.NewRecorder` to capture responses
65-
4. Validating status codes, headers, and response bodies
105+
1. Create mock dependencies
106+
2. Set up HTTP requests with `httptest.NewRequest`
107+
3. Use `httptest.NewRecorder` to capture responses
108+
4. Validate status codes, headers, and response bodies
66109

67110
### Controllers (Business Logic)
68-
69-
Test controllers by:
70-
1. Mocking all dependencies (managers, utilities, etc.)
71-
2. Setting up mock expectations with `mock.On()` and `mock.InOrder()`
72-
3. Testing both success and error paths
73-
4. Verifying mock expectations with `AssertExpectations(t)`
74-
75-
### Types and Data Structures
76-
77-
Test types by:
78-
1. Validating serialization/deserialization (JSON marshaling)
79-
2. Testing validation methods
80-
3. Testing error handling and edge cases
81-
4. Using table-driven tests for multiple validation scenarios
111+
1. Mock all dependencies (managers, utilities, etc.)
112+
2. Set up mock expectations with `mock.On()` and `mock.InOrder()`
113+
3. Test both success and error paths
114+
4. Verify mock expectations with `AssertExpectations(t)`
82115

83116
### Integration Tests
117+
1. Create real API instances with test configurations
118+
2. Use `httptest.NewServer` for full HTTP testing
119+
3. Test authentication flows end-to-end
120+
4. Validate complete request/response cycles
84121

85-
Structure integration tests by:
86-
1. Creating real API instances with test configurations
87-
2. Using `httptest.NewServer` for full HTTP testing
88-
3. Testing authentication flows end-to-end
89-
4. Validating complete request/response cycles
90-
5. Testing with both direct HTTP calls and API client libraries
122+
## HTTP Testing
91123

92-
## Mock Usage
124+
- **Unit-level**: Use `net/http/httptest` for testing individual handlers
125+
- **Integration**: Create full API instances with `httptest.NewServer`
126+
- **API Client**: Test both direct HTTP calls and API client wrapper methods
93127

94-
### Mock Setup Patterns
128+
## What NOT to Test
95129

96-
- Use setup functions in table tests: `setupMock func(*MockType)`
97-
- Use `mock.InOrder()` for sequential mock calls
98-
- Use `mock.MatchedBy()` for complex argument matching
99-
- Always call `AssertExpectations(t)` to verify all mocks were called
130+
- Data structure existence: `assert.NotNil(t, &Config{})`
131+
- Private implementation details
132+
- External library behavior
133+
- Tests that don't call any functions or validate actual behavior
100134

101-
### Mock Lifecycle
135+
## Required Assertions
102136

103-
- Create fresh mocks for each test case
104-
- Set up mock expectations before running the test
105-
- Verify mock expectations after the test completes
106-
107-
## Test Structure
108-
109-
### Test Naming
110-
- Use table-driven tests for multiple scenarios with descriptive test case names
111-
- Test functions follow pattern: `TestFunctionName` for simple cases, `TestTypeName_MethodName` for methods
112-
- Test case names in tables should be descriptive: `name: "empty data directory should return error"`
113-
- Example of proper table-driven test structure:
114-
```go
115-
func TestValidateConfig(t *testing.T) {
116-
tests := []struct {
117-
name string
118-
input *Config
119-
expectedError string
120-
}{
121-
{
122-
name: "empty data directory should return error",
123-
input: &Config{DataDirectory: ""},
124-
expectedError: "data directory cannot be empty",
125-
},
126-
{
127-
name: "valid directory should succeed",
128-
input: &Config{DataDirectory: "/opt/data"},
129-
expectedError: "",
130-
},
131-
}
132-
for _, tt := range tests {
133-
t.Run(tt.name, func(t *testing.T) {
134-
err := ValidateConfig(tt.input)
135-
// test logic here
136-
})
137-
}
138-
}
139-
```
140-
141-
### Use Semantic Naming for Test Cases
142-
143-
- **Name test cases descriptively**: Use meaningful names for test cases rather than generic names like "test case 1"
144-
```go
145-
tests := []struct {
146-
name string
147-
input *Config
148-
expectedError string
149-
}{
150-
{
151-
name: "empty data directory should return error",
152-
input: &Config{DataDirectory: ""},
153-
expectedError: "data directory cannot be empty",
154-
},
155-
{
156-
name: "test case 2",
157-
input: &Config{DataDirectory: "/opt/data"},
158-
expectedError: "",
159-
},
160-
}
161-
```
162-
163-
### Test Organization
164-
- Create one test file per source file (`file.go` → `file_test.go`)
165-
- Use subtests for variations within table-driven tests
166-
- Keep test files focused on testing their corresponding source file
167-
168-
### Test Coverage and Quality
169-
170-
- **Remove tests that don't call any functions or validate actual behavior**: Tests should assert on real functionality, not just test data structures
171-
```go
172-
func TestValidateConfig_EmptyDirectory_ReturnsError(t *testing.T) {
173-
config := &Config{DataDirectory: ""}
174-
err := ValidateConfig(config) // Actually calls the function
175-
assert.Error(t, err)
176-
}
177-
178-
func TestConfigStruct_Fields_Exist(t *testing.T) {
179-
config := &Config{}
180-
assert.NotNil(t, config) // Just tests data structure
181-
}
182-
```
137+
- Use `require.*` for fatal assertions (stop test on failure)
138+
- Use `assert.*` for non-fatal assertions (continue test)
139+
- Always call `AssertExpectations(t)` on mocks
183140

184-
### Mock Management
141+
## Test Quality Rules
185142

186-
- **Reuse existing mock interfaces across tests**: Use the same mock type for the same interface rather than creating multiple mock types
187-
```go
188-
// Good - reuse the same mock across different tests
189-
type MockInstallationManager struct {
190-
mock.Mock
191-
}
192-
func (m *MockInstallationManager) GetConfig() (Config, error) { ... }
193-
func (m *MockInstallationManager) SetConfig(config Config) error { ... }
194-
195-
// Use this mock in multiple test files for the same interface
196-
197-
// Bad - creating separate mocks for the same interface
198-
type MockInstallManager struct { ... } // for installation tests
199-
type MockConfigManager struct { ... } // for config tests
200-
// Both implementing the same InstallationManager interface
201-
```
202-
- **Follow consistent mock patterns**: All mocks use `testify/mock` with `type MockTypeName struct { mock.Mock }`
203-
- **Implement complete interfaces**: Mock all methods of the interface even if some tests don't use them
143+
- **Test for behavior, not implementation**: Verify public behavior, not internal details
144+
- **Readable and maintainable**: Tests should serve as documentation
145+
- **Fast and reliable**: Tests should run quickly and produce consistent results
146+
- **Remove meaningless tests**: Tests should assert on real functionality, not just data structures
204147

‎api/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@ The root directory contains the main API setup files and request handlers.
1010
### Subpackages
1111

1212
#### `/controllers`
13-
Contains the business logic for different API endpoints. Each controller package focuses on a specific domain of functionality or workflow (e.g., authentication, console, install, upgrade, join, etc.) and implements the core business logic for that domain or workflow. Controllers can utilize multiple managers with each manager handling a specific subdomain of functionality.
13+
Contains the business logic for different API endpoints. Each controller package focuses on a specific domain of functionality or workflow (e.g., authentication, console, install, upgrade, join, etc.) and implements the core business logic for that domain or workflow. Controllers can utilize multiple managers with each manager handling a specific subdomain of functionality. Controllers act as orchestrators that read from one manager and pass data to another - never inject managers into other managers.
1414

1515
#### `/internal`
1616
Contains shared utilities and helper packages that provide common functionality used across different parts of the API. This includes both general-purpose utilities and domain-specific helpers.
1717

1818
#### `/internal/managers`
19-
Each manager is responsible for a specific subdomain of functionality and provides a clean interface for controllers to interact with. For example, the Preflight Manager manages system requirement checks and validation.
19+
Each manager is responsible for a specific subdomain of functionality and provides a clean interface for controllers to interact with. For example, the Preflight Manager manages system requirement checks and validation. Managers must remain independent with no cross-manager dependencies - data flows between managers through the controller, not directly between managers.
2020

2121
#### `/internal/statemachine`
2222
The statemachine is used by controllers to capture workflow state and enforce valid transitions.

0 commit comments

Comments
 (0)