Skip to content

Commit 2cc3bfd

Browse files
JAORMXclaude
andauthored
Refactor CA certificate config operations to match registry pattern (#2476)
This change moves CA certificate configuration operations from CLI command functions into the pkg/config package, following the established pattern used by registry operations. Changes: - Add pkg/config/cacert.go with helper functions (setCACert, getCACert, unsetCACert) - Extend Provider interface with CA cert methods - Implement CA cert methods in DefaultProvider, PathProvider, and KubernetesProvider - Refactor CLI commands to use provider methods, reducing CLI code by 60% - Add comprehensive table-driven tests (9 test cases) This establishes consistent patterns across all config operations and improves separation of concerns by moving validation logic out of the CLI layer. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
1 parent 048598c commit 2cc3bfd

File tree

5 files changed

+471
-40
lines changed

5 files changed

+471
-40
lines changed

cmd/thv/app/config.go

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77

88
"github.com/spf13/cobra"
99

10-
"github.com/stacklok/toolhive/pkg/certs"
1110
"github.com/stacklok/toolhive/pkg/config"
1211
)
1312

@@ -99,73 +98,51 @@ func init() {
9998
}
10099

101100
func setCACertCmdFunc(_ *cobra.Command, args []string) error {
102-
certPath := filepath.Clean(args[0])
101+
certPath := args[0]
103102

104-
// Validate that the file exists and is readable
105-
if _, err := os.Stat(certPath); err != nil {
106-
return fmt.Errorf("CA certificate file not found or not accessible: %w", err)
107-
}
108-
109-
// Read and validate the certificate
110-
certContent, err := os.ReadFile(certPath)
111-
if err != nil {
112-
return fmt.Errorf("failed to read CA certificate file: %w", err)
113-
}
114-
115-
// Validate the certificate format
116-
if err := certs.ValidateCACertificate(certContent); err != nil {
117-
return fmt.Errorf("invalid CA certificate: %w", err)
118-
}
119-
120-
// Update the configuration
121-
err = config.UpdateConfig(func(c *config.Config) {
122-
c.CACertificatePath = certPath
123-
})
103+
provider := config.NewDefaultProvider()
104+
err := provider.SetCACert(certPath)
124105
if err != nil {
125-
return fmt.Errorf("failed to update configuration: %w", err)
106+
return err
126107
}
127108

128-
fmt.Printf("Successfully set CA certificate path: %s\n", certPath)
109+
fmt.Printf("Successfully set CA certificate path: %s\n", filepath.Clean(certPath))
129110
return nil
130111
}
131112

132113
func getCACertCmdFunc(_ *cobra.Command, _ []string) error {
133-
configProvider := config.NewDefaultProvider()
134-
cfg := configProvider.GetConfig()
114+
provider := config.NewDefaultProvider()
115+
certPath, exists, accessible := provider.GetCACert()
135116

136-
if cfg.CACertificatePath == "" {
117+
if !exists {
137118
fmt.Println("No CA certificate is currently configured.")
138119
return nil
139120
}
140121

141-
fmt.Printf("Current CA certificate path: %s\n", cfg.CACertificatePath)
122+
fmt.Printf("Current CA certificate path: %s\n", certPath)
142123

143-
// Check if the file still exists
144-
if _, err := os.Stat(cfg.CACertificatePath); err != nil {
145-
fmt.Printf("Warning: The configured CA certificate file is not accessible: %v\n", err)
124+
if !accessible {
125+
fmt.Printf("Warning: The configured CA certificate file is not accessible\n")
146126
}
147127

148128
return nil
149129
}
150130

151131
func unsetCACertCmdFunc(_ *cobra.Command, _ []string) error {
152-
configProvider := config.NewDefaultProvider()
153-
cfg := configProvider.GetConfig()
132+
provider := config.NewDefaultProvider()
133+
certPath, exists, _ := provider.GetCACert()
154134

155-
if cfg.CACertificatePath == "" {
135+
if !exists {
156136
fmt.Println("No CA certificate is currently configured.")
157137
return nil
158138
}
159139

160-
// Update the configuration
161-
err := config.UpdateConfig(func(c *config.Config) {
162-
c.CACertificatePath = ""
163-
})
140+
err := provider.UnsetCACert()
164141
if err != nil {
165-
return fmt.Errorf("failed to update configuration: %w", err)
142+
return err
166143
}
167144

168-
fmt.Println("Successfully removed CA certificate configuration.")
145+
fmt.Printf("Successfully removed CA certificate configuration: %s\n", certPath)
169146
return nil
170147
}
171148

pkg/config/cacert.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package config
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"path/filepath"
7+
8+
"github.com/stacklok/toolhive/pkg/certs"
9+
)
10+
11+
// setCACert validates and sets the CA certificate path using the provided provider.
12+
// It performs the following validations:
13+
// - Verifies the file exists and is readable
14+
// - Reads the certificate content
15+
// - Validates the certificate format using pkg/certs.ValidateCACertificate
16+
// - Cleans the file path
17+
//
18+
// The function returns an error if any validation fails or if updating the configuration fails.
19+
func setCACert(provider Provider, certPath string) error {
20+
// Clean the path
21+
certPath = filepath.Clean(certPath)
22+
23+
// Validate that the file exists and is readable
24+
if _, err := os.Stat(certPath); err != nil {
25+
return fmt.Errorf("CA certificate file not found or not accessible: %w", err)
26+
}
27+
28+
// Read and validate the certificate
29+
certContent, err := os.ReadFile(certPath)
30+
if err != nil {
31+
return fmt.Errorf("failed to read CA certificate file: %w", err)
32+
}
33+
34+
// Validate the certificate format
35+
if err := certs.ValidateCACertificate(certContent); err != nil {
36+
return fmt.Errorf("invalid CA certificate: %w", err)
37+
}
38+
39+
// Update the configuration
40+
err = provider.UpdateConfig(func(c *Config) {
41+
c.CACertificatePath = certPath
42+
})
43+
if err != nil {
44+
return fmt.Errorf("failed to update configuration: %w", err)
45+
}
46+
47+
return nil
48+
}
49+
50+
// getCACert returns the currently configured CA certificate path and its accessibility status.
51+
// It returns three values:
52+
// - certPath: the configured certificate path (empty string if not set)
53+
// - exists: true if a CA certificate is configured in the config
54+
// - accessible: true if the certificate file exists and is accessible on the filesystem
55+
//
56+
// Note: exists can be true while accessible is false if the file was deleted after configuration.
57+
func getCACert(provider Provider) (certPath string, exists bool, accessible bool) {
58+
cfg := provider.GetConfig()
59+
60+
if cfg.CACertificatePath == "" {
61+
return "", false, false
62+
}
63+
64+
certPath = cfg.CACertificatePath
65+
exists = true
66+
67+
// Check if the file is still accessible
68+
if _, err := os.Stat(certPath); err != nil {
69+
accessible = false
70+
} else {
71+
accessible = true
72+
}
73+
74+
return certPath, exists, accessible
75+
}
76+
77+
// unsetCACert removes the CA certificate configuration from the config file.
78+
// If no CA certificate is currently configured, this function is a no-op and returns nil.
79+
// Returns an error if updating the configuration fails.
80+
func unsetCACert(provider Provider) error {
81+
cfg := provider.GetConfig()
82+
83+
if cfg.CACertificatePath == "" {
84+
// Already unset, no-op
85+
return nil
86+
}
87+
88+
// Update the configuration
89+
err := provider.UpdateConfig(func(c *Config) {
90+
c.CACertificatePath = ""
91+
})
92+
if err != nil {
93+
return fmt.Errorf("failed to update configuration: %w", err)
94+
}
95+
96+
return nil
97+
}

0 commit comments

Comments
 (0)