diff --git a/AGENTS.md b/AGENTS.md index 1d824141..9a2d41ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,92 +48,6 @@ When adding a new tool: 3. Add the tool to an appropriate toolset (or create a new toolset if needed). 4. Register the toolset in `pkg/toolsets/` if it's a new toolset. -### Configuration Standards for Red Hat-Maintained Components - -To ensure consistency across Red Hat-maintained toolsets and cluster providers, follow these standards when implementing certificate authority (CA) configuration: - -#### Certificate Authority (CA) Configuration Standard - -**Standard Pattern:** Certificate authorities must be configured using **file paths**, not inline PEM content. - -**Implementation Guidelines:** - -1. **Config Parser (`pkg/toolsets//config.go`):** - - Accept `certificate_authority` as a string field in your toolset config struct. - - In the toolset parser function, resolve relative paths using `config.ConfigDirPathFromContext(ctx)` to make them relative to the config file directory. - - Store the resolved absolute path in the config struct. - - Validate in the `Validate()` method that `certificate_authority` is a valid file path using `os.Stat()`. - - Example: - ```go - func (c *Config) Validate() error { - // ... other validations ... - - // Validate that certificate_authority is a valid file - if caValue := strings.TrimSpace(c.CertificateAuthority); caValue != "" { - if _, err := os.Stat(caValue); err != nil { - return fmt.Errorf("certificate_authority must be a valid file path: %w", err) - } - } - return nil - } - - func toolsetParser(ctx context.Context, primitive toml.Primitive, md toml.MetaData) (config.Extended, error) { - var cfg Config - if err := md.PrimitiveDecode(primitive, &cfg); err != nil { - return nil, err - } - - // Resolve relative CA file paths - if cfg.CertificateAuthority != "" { - configDir := config.ConfigDirPathFromContext(ctx) - if configDir != "" && !filepath.IsAbs(cfg.CertificateAuthority) { - cfg.CertificateAuthority = filepath.Join(configDir, cfg.CertificateAuthority) - } - } - - return &cfg, nil - } - ``` - -2. **Client Implementation:** - - When using the CA certificate, read it from the file path using `os.ReadFile()`. - - Only valid file paths are supported; the path must exist and be readable. - - Example: - ```go - func (c *Client) createHTTPClient() *http.Client { - tlsConfig := &tls.Config{InsecureSkipVerify: c.insecure} - - if caValue := strings.TrimSpace(c.certificateAuthority); caValue != "" { - // Read the certificate from file - caPEM, err := os.ReadFile(caValue) - if err != nil { - // Handle error appropriately - } - - // Use caPEM to configure TLS... - } - - return &http.Client{Transport: &http.Transport{TLSClientConfig: tlsConfig}} - } - ``` - -3. **Documentation:** - - Document that `certificate_authority` accepts only a file path (absolute or relative). - - Mention that relative paths are resolved relative to the config file directory. - - Note that the file path must exist and be readable; invalid paths will cause validation to fail. - -**Reference Implementation:** -- See `pkg/kiali/config.go` and `pkg/kiali/kiali.go` for a complete example. -- See `pkg/kubernetes-mcp-server/cmd/root.go` (lines 292-314) for how the main server handles CA files. - -**Rationale:** -- Consistency with ACM cluster provider and other Red Hat-maintained components. -- Better security practices (certificates stored in files, not in config files). -- Easier management and rotation of certificates. -- Clearer separation of configuration and secrets. - ## Building Use the provided Makefile targets: diff --git a/pkg/config/config.go b/pkg/config/config.go index 88036cbc..85bf74d0 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -89,17 +89,13 @@ type GroupVersionKind struct { type ReadConfigOpt func(cfg *StaticConfig) -func withDirPath(path string) ReadConfigOpt { +// WithDirPath returns a ReadConfigOpt that sets the config directory path. +func WithDirPath(path string) ReadConfigOpt { return func(cfg *StaticConfig) { cfg.configDirPath = path } } -// WithDirPath returns a ReadConfigOpt that sets the config directory path. -func WithDirPath(path string) ReadConfigOpt { - return withDirPath(path) -} - // Read reads the toml file and returns the StaticConfig, with any opts applied. func Read(configPath string, opts ...ReadConfigOpt) (*StaticConfig, error) { configData, err := os.ReadFile(configPath) @@ -114,7 +110,7 @@ func Read(configPath string, opts ...ReadConfigOpt) (*StaticConfig, error) { } dirPath := filepath.Dir(absPath) - cfg, err := ReadToml(configData, append(opts, withDirPath(dirPath))...) + cfg, err := ReadToml(configData, append(opts, WithDirPath(dirPath))...) if err != nil { return nil, err } diff --git a/pkg/kiali/config_test.go b/pkg/kiali/config_test.go index c37fd7de..5c89934a 100644 --- a/pkg/kiali/config_test.go +++ b/pkg/kiali/config_test.go @@ -17,23 +17,14 @@ type ConfigSuite struct { } func (s *ConfigSuite) SetupTest() { - // Create a temporary directory for test files - tempDir, err := os.MkdirTemp("", "kiali-config-test-*") - s.Require().NoError(err, "Failed to create temp directory") - s.tempDir = tempDir - // Create a test CA certificate file + s.tempDir = s.T().TempDir() s.caFile = filepath.Join(s.tempDir, "ca.crt") - err = os.WriteFile(s.caFile, []byte("test ca content"), 0644) + err := os.WriteFile(s.caFile, []byte("test ca content"), 0644) s.Require().NoError(err, "Failed to write CA file") } func (s *ConfigSuite) TestConfigParser_ResolvesRelativePath() { - // Create CA file in temp directory - caFile := filepath.Join(s.tempDir, "ca.crt") - err := os.WriteFile(caFile, []byte("test ca content"), 0644) - s.Require().NoError(err, "Failed to write CA file") - // Read config with configDirPath set to tempDir to resolve relative paths cfg := test.Must(config.ReadToml([]byte(` [toolset_configs.kiali] @@ -48,8 +39,7 @@ func (s *ConfigSuite) TestConfigParser_ResolvesRelativePath() { s.Require().True(ok, "Kiali config should be of type *Config") // Verify the path was resolved to absolute - expectedPath := caFile - s.Equal(expectedPath, kcfg.CertificateAuthority, "Relative path should be resolved to absolute path") + s.Equal(s.caFile, kcfg.CertificateAuthority, "Relative path should be resolved to absolute path") } func (s *ConfigSuite) TestConfigParser_PreservesAbsolutePath() {