feat(kiali): standardize Certificate Authority Configuration Method (#511)

* Get certificate by path as well

Signed-off-by: josunect <jcordoba@redhat.com>

* Remove inline certs support

Signed-off-by: josunect <jcordoba@redhat.com>

* Fix for windows

Signed-off-by: josunect <jcordoba@redhat.com>

* Update failing test

Signed-off-by: josunect <jcordoba@redhat.com>

* Unify test usages

Signed-off-by: josunect <jcordoba@redhat.com>

* Remove PEM from docs

Signed-off-by: josunect <jcordoba@redhat.com>

* Missing doc

Signed-off-by: josunect <jcordoba@redhat.com>

* Remove PEM checks

Signed-off-by: josunect <jcordoba@redhat.com>

* Lint

Signed-off-by: josunect <jcordoba@redhat.com>

* Update docs

Signed-off-by: josunect <jcordoba@redhat.com>

* Update test

Signed-off-by: josunect <jcordoba@redhat.com>

* Comments from review

Signed-off-by: josunect <jcordoba@redhat.com>

* Update pkg/config/config.go

Co-authored-by: Alberto Gutierrez <aljesusg@gmail.com>
Signed-off-by: josunect <jcordoba@redhat.com>

---------

Signed-off-by: josunect <jcordoba@redhat.com>
Co-authored-by: Alberto Gutierrez <aljesusg@gmail.com>

Author: josunect

AGENTS.md

@@ -48,6 +48,92 @@ 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/<toolset>/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:

docs/KIALI.md

@@ -14,9 +14,7 @@ toolsets = ["core", "kiali"]
 [toolset_configs.kiali]
 url = "https://kiali.example" # Endpoint/route to reach Kiali console
 # insecure = true  # optional: allow insecure TLS (not recommended in production)
-# certificate_authority = """-----BEGIN CERTIFICATE-----
-# MIID...
-# -----END CERTIFICATE-----"""
+# certificate_authority = "/path/to/ca.crt"  # File path to CA certificate
 # When url is https and insecure is false, certificate_authority is required.
 ```
 
@@ -32,6 +30,6 @@ When the `kiali` toolset is enabled, a Kiali toolset configuration is required v
 - Missing Kiali configuration when `kiali` toolset is enabled → set `[toolset_configs.kiali].url` in the config TOML.
 - Invalid URL → ensure `[toolset_configs.kiali].url` is a valid `http(s)://host` URL.
 - TLS certificate validation:
-  - If `[toolset_configs.kiali].url` uses HTTPS and `[toolset_configs.kiali].insecure` is false, you must set `[toolset_configs.kiali].certificate_authority` with the PEM-encoded certificate(s) used by the Kiali server. This field expects inline PEM content, not a file path. You may concatenate multiple PEM blocks to include an intermediate chain.
+  - If `[toolset_configs.kiali].url` uses HTTPS and `[toolset_configs.kiali].insecure` is false, you must set `[toolset_configs.kiali].certificate_authority` with the path to the CA certificate file. Relative paths are resolved relative to the directory containing the config file.
   - For non-production environments you can set `[toolset_configs.kiali].insecure = true` to skip certificate verification.
 

pkg/config/config.go

@@ -95,6 +95,11 @@ func withDirPath(path string) ReadConfigOpt {
 	}
 }
 
+// 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)

pkg/kiali/config.go

@@ -3,7 +3,10 @@ package kiali
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net/url"
+	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/BurntSushi/toml"
@@ -33,14 +36,30 @@ func (c *Config) Validate() error {
 	if strings.EqualFold(u.Scheme, "https") && !c.Insecure && strings.TrimSpace(c.CertificateAuthority) == "" {
 		return errors.New("certificate_authority is required for https when insecure is false")
 	}
+	// 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 kialiToolsetParser(_ context.Context, primitive toml.Primitive, md toml.MetaData) (config.Extended, error) {
+func kialiToolsetParser(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
 	}
+
+	// If certificate_authority is provided, resolve it relative to the config directory if it's a relative path
+	if cfg.CertificateAuthority != "" {
+		configDir := config.ConfigDirPathFromContext(ctx)
+		if configDir != "" && !filepath.IsAbs(cfg.CertificateAuthority) {
+			cfg.CertificateAuthority = filepath.Join(configDir, cfg.CertificateAuthority)
+		}
+		// If it's already absolute or configDir is empty, use as-is
+	}
+
 	return &cfg, nil
 }
 

pkg/kiali/config_test.go

@@ -0,0 +1,96 @@
+package kiali
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/containers/kubernetes-mcp-server/internal/test"
+	"github.com/containers/kubernetes-mcp-server/pkg/config"
+	"github.com/stretchr/testify/suite"
+)
+
+type ConfigSuite struct {
+	suite.Suite
+	tempDir string
+	caFile  string
+}
+
+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.caFile = filepath.Join(s.tempDir, "ca.crt")
+	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]
+		url = "https://kiali.example/"
+		certificate_authority = "ca.crt"
+	`), config.WithDirPath(s.tempDir)))
+
+	// Get Kiali config
+	kialiCfg, ok := cfg.GetToolsetConfig("kiali")
+	s.Require().True(ok, "Kiali config should be present")
+	kcfg, ok := kialiCfg.(*Config)
+	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")
+}
+
+func (s *ConfigSuite) TestConfigParser_PreservesAbsolutePath() {
+	// Convert backslashes to forward slashes for TOML compatibility on Windows
+	caFileForTOML := filepath.ToSlash(s.caFile)
+
+	cfg := test.Must(config.ReadToml([]byte(`
+		[toolset_configs.kiali]
+		url = "https://kiali.example/"
+		certificate_authority = "` + caFileForTOML + `"
+	`)))
+
+	kialiCfg, ok := cfg.GetToolsetConfig("kiali")
+	s.Require().True(ok, "Kiali config should be present")
+	kcfg, ok := kialiCfg.(*Config)
+	s.Require().True(ok, "Kiali config should be of type *Config")
+
+	// Absolute path should be preserved
+	actualPath := filepath.Clean(filepath.FromSlash(kcfg.CertificateAuthority))
+	expectedPath := filepath.Clean(s.caFile)
+	s.Equal(expectedPath, actualPath, "Absolute path should be preserved")
+}
+
+func (s *ConfigSuite) TestConfigParser_RejectsInvalidFile() {
+	// Use a non-existent file path
+	nonExistentFile := filepath.Join(s.tempDir, "non-existent.crt")
+	// Convert backslashes to forward slashes for TOML compatibility on Windows
+	nonExistentFileForTOML := filepath.ToSlash(nonExistentFile)
+
+	cfg, err := config.ReadToml([]byte(`
+		[toolset_configs.kiali]
+		url = "https://kiali.example/"
+		certificate_authority = "` + nonExistentFileForTOML + `"
+	`))
+
+	// Validate should reject invalid file path
+	s.Require().Error(err, "Validate should reject invalid file path")
+	s.Contains(err.Error(), "certificate_authority must be a valid file path", "Error message should indicate file path is invalid")
+	s.Nil(cfg, "Config should be nil when validation fails")
+}
+
+func TestConfig(t *testing.T) {
+	suite.Run(t, new(ConfigSuite))
+}

pkg/kiali/kiali.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"os"
 	"strings"
 
 	"github.com/containers/kubernetes-mcp-server/pkg/config"
@@ -85,19 +86,30 @@ func (k *Kiali) createHTTPClient() *http.Client {
 		InsecureSkipVerify: k.kialiInsecure,
 	}
 
-	// If a custom Certificate Authority PEM is configured, load and add it
-	if caPEM := strings.TrimSpace(k.certificateAuthority); caPEM != "" {
+	// If a custom Certificate Authority is configured, load and add it
+	if caValue := strings.TrimSpace(k.certificateAuthority); caValue != "" {
+		// Read the certificate from file
+		caPEM, err := os.ReadFile(caValue)
+		if err != nil {
+			klog.Errorf("failed to read CA certificate from file %s: %v; proceeding without custom CA", caValue, err)
+			return &http.Client{
+				Transport: &http.Transport{
+					TLSClientConfig: tlsConfig,
+				},
+			}
+		}
+
 		// Start with the host system pool when possible so we don't drop system roots
 		var certPool *x509.CertPool
 		if systemPool, err := x509.SystemCertPool(); err == nil && systemPool != nil {
 			certPool = systemPool
 		} else {
 			certPool = x509.NewCertPool()
 		}
-		if ok := certPool.AppendCertsFromPEM([]byte(caPEM)); ok {
+		if ok := certPool.AppendCertsFromPEM(caPEM); ok {
 			tlsConfig.RootCAs = certPool
 		} else {
-			klog.V(0).Infof("failed to append provided certificate authority PEM; proceeding without custom CA")
+			klog.V(0).Infof("failed to append provided certificate authority; proceeding without custom CA")
 		}
 	}
 

pkg/kubernetes-mcp-server/cmd/root.go

@@ -262,6 +262,12 @@ func (m *MCPServerOptions) Validate() error {
 			klog.Warningf("authorization-url is using http://, this is not recommended production use")
 		}
 	}
+	// Validate that certificate_authority is a valid file
+	if caValue := strings.TrimSpace(m.StaticConfig.CertificateAuthority); caValue != "" {
+		if _, err := os.Stat(caValue); err != nil {
+			return fmt.Errorf("certificate-authority must be a valid file path: %w", err)
+		}
+	}
 	return nil
 }