Skip to content

Commit 739cf62

Browse files
committed
simplfy logic
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
1 parent 6422b1c commit 739cf62

5 files changed

Lines changed: 24 additions & 110 deletions

File tree

app/cli/cmd/config_save.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2023-2025 The Chainloop Authors.
2+
// Copyright 2023-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -30,8 +30,12 @@ func newConfigSaveCmd() *cobra.Command {
3030
},
3131
RunE: func(cmd *cobra.Command, args []string) error {
3232
// Process CA flags - read file contents and encode to base64 if needed
33-
processCAFlag(confOptions.controlplaneCA)
34-
processCAFlag(confOptions.CASCA)
33+
if err := processCAFlag(confOptions.controlplaneCA); err != nil {
34+
return err
35+
}
36+
if err := processCAFlag(confOptions.CASCA); err != nil {
37+
return err
38+
}
3539
return viper.WriteConfig()
3640
},
3741
}

app/cli/cmd/root.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2024-2025 The Chainloop Authors.
2+
// Copyright 2024-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -125,8 +125,8 @@ func NewRootCmd(l zerolog.Logger) *cobra.Command {
125125
}
126126

127127
if caValue := viper.GetString(confOptions.controlplaneCA.viperKey); caValue != "" {
128-
// Check if it's a file path or base64/PEM content
129-
if grpcconn.IsFilePath(caValue) {
128+
// Check if the value is a file path, if it is we read the content and encode it to base64, if not we assume it's the content already
129+
if _, err := os.Stat(caValue); err == nil {
130130
opts = append(opts, grpcconn.WithCAFile(caValue))
131131
} else {
132132
opts = append(opts, grpcconn.WithCAContent(caValue))
@@ -503,23 +503,23 @@ func getConfigDir(appName string) string {
503503
}
504504

505505
// processCAFlag reads CA file content and encodes it to base64 if value is a file path
506-
func processCAFlag(opt *confOpt) {
506+
func processCAFlag(opt *confOpt) error {
507507
value := viper.GetString(opt.viperKey)
508508
if value == "" {
509-
return
509+
return nil
510510
}
511511

512512
// If it's a file path, read and encode
513-
if grpcconn.IsFilePath(value) {
513+
if _, err := os.Stat(value); err == nil {
514514
content, err := os.ReadFile(value)
515515
if err != nil {
516-
// Log warning but don't fail
517-
logger.Warn().Err(err).Str("file", value).Msg("Failed to read CA file")
518-
return
516+
return fmt.Errorf("failed to read CA file %s: %w", value, err)
519517
}
520518

521519
// Store base64-encoded content in viper (will be persisted by config save)
522520
encoded := base64.StdEncoding.EncodeToString(content)
523521
viper.Set(opt.viperKey, encoded)
524522
}
523+
524+
return nil
525525
}

app/cli/pkg/action/action.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2024-2025 The Chainloop Authors.
2+
// Copyright 2024-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -142,8 +142,8 @@ func getCASBackend(ctx context.Context, client pb.AttestationServiceClient, work
142142

143143
opts := []grpcconn.Option{grpcconn.WithInsecure(casConnectionInsecure)}
144144
if casCAPath != "" {
145-
// Check if it's a file path or base64/PEM content
146-
if grpcconn.IsFilePath(casCAPath) {
145+
// Check if it's a file path or content. If it's a file path, it should exist. If not, treat it as content.
146+
if _, err := os.Stat(casCAPath); err == nil {
147147
opts = append(opts, grpcconn.WithCAFile(casCAPath))
148148
} else {
149149
opts = append(opts, grpcconn.WithCAContent(casCAPath))

pkg/grpcconn/grpcconn.go

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2024-2025 The Chainloop Authors.
2+
// Copyright 2024-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -21,7 +21,6 @@ import (
2121
"encoding/base64"
2222
"fmt"
2323
"os"
24-
"strings"
2524

2625
grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
2726
"google.golang.org/grpc"
@@ -177,30 +176,3 @@ func (t tokenAuth) GetRequestMetadata(_ context.Context, _ ...string) (map[strin
177176
func (t tokenAuth) RequireTransportSecurity() bool {
178177
return !t.insecure
179178
}
180-
181-
// IsFilePath checks if a value looks like a file path (vs base64/PEM content).
182-
// It returns true if the value appears to be a file path, false if it appears to be content.
183-
func IsFilePath(value string) bool {
184-
// Check if value looks like a file path
185-
// If it starts with /, ./, ../, or ~/, it's a path
186-
// If it contains a newline, it's likely PEM content
187-
// If it's valid base64 without path separators, assume it's content
188-
189-
if strings.HasPrefix(value, "/") ||
190-
strings.HasPrefix(value, "./") ||
191-
strings.HasPrefix(value, "../") ||
192-
strings.HasPrefix(value, "~/") {
193-
return true
194-
}
195-
196-
if strings.Contains(value, "\n") {
197-
return false // PEM content
198-
}
199-
200-
// Check if file exists
201-
if _, err := os.Stat(value); err == nil {
202-
return true
203-
}
204-
205-
return false
206-
}

pkg/grpcconn/grpcconn_test.go

Lines changed: 4 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"crypto/x509"
2121
"encoding/base64"
2222
"os"
23-
"path/filepath"
2423
"testing"
2524

2625
"github.com/stretchr/testify/assert"
@@ -54,68 +53,6 @@ func TestRequireTransportSecurity(t *testing.T) {
5453
}
5554
}
5655

57-
func TestIsFilePath(t *testing.T) {
58-
// Create a temporary file for testing
59-
tmpDir := t.TempDir()
60-
tmpFile := filepath.Join(tmpDir, "test.pem")
61-
err := os.WriteFile(tmpFile, []byte("test content"), 0600)
62-
require.NoError(t, err)
63-
64-
testCases := []struct {
65-
name string
66-
value string
67-
want bool
68-
}{
69-
{
70-
name: "absolute path",
71-
value: "/path/to/ca.pem",
72-
want: true,
73-
},
74-
{
75-
name: "relative path with ./",
76-
value: "./ca.pem",
77-
want: true,
78-
},
79-
{
80-
name: "relative path with ../",
81-
value: "../ca.pem",
82-
want: true,
83-
},
84-
{
85-
name: "home directory path",
86-
value: "~/ca.pem",
87-
want: true,
88-
},
89-
{
90-
name: "existing file",
91-
value: tmpFile,
92-
want: true,
93-
},
94-
{
95-
name: "PEM content with newlines",
96-
value: "-----BEGIN CERTIFICATE-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A\n-----END CERTIFICATE-----",
97-
want: false,
98-
},
99-
{
100-
name: "base64 encoded content",
101-
value: "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUJJakFOQmdrcWhraUc5dzBCQVFFRkFBT0NBUThBCi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0=",
102-
want: false,
103-
},
104-
{
105-
name: "non-existent file without path prefix",
106-
value: "nonexistent.pem",
107-
want: false,
108-
},
109-
}
110-
111-
for _, tc := range testCases {
112-
t.Run(tc.name, func(t *testing.T) {
113-
got := IsFilePath(tc.value)
114-
assert.Equal(t, tc.want, got, "IsFilePath(%q) = %v, want %v", tc.value, got, tc.want)
115-
})
116-
}
117-
}
118-
11956
func TestAppendCAFromFile(t *testing.T) {
12057
// Check if the file exists, skip test if not
12158
if _, err := os.Stat(caPath); os.IsNotExist(err) {
@@ -210,7 +147,8 @@ func TestBackwardCompatibility_StoredFilePath(t *testing.T) {
210147
storedValue := caPath
211148

212149
// Verify IsFilePath detects it as a file path
213-
assert.True(t, IsFilePath(storedValue), "stored file path should be detected as a file path")
150+
_, err := os.Stat(storedValue)
151+
assert.Nil(t, err, "stored file path should be detected as a file path")
214152

215153
// Verify it can be loaded using the file path method
216154
certsPool, err := x509.SystemCertPool()
@@ -233,7 +171,7 @@ func TestBackwardCompatibility_NewClientOldConfig(t *testing.T) {
233171

234172
// New client logic: detect and load appropriately
235173
var opts []Option
236-
if IsFilePath(oldConfigValue) {
174+
if _, err := os.Stat(oldConfigValue); err == nil {
237175
opts = append(opts, WithCAFile(oldConfigValue))
238176
} else {
239177
opts = append(opts, WithCAContent(oldConfigValue))
@@ -273,7 +211,7 @@ func TestBackwardCompatibility_OldClientNewConfig(t *testing.T) {
273211
assert.NoError(t, err, "old client should load file path")
274212

275213
// New client behavior: detect then use file path
276-
if IsFilePath(configValue) {
214+
if _, statErr := os.Stat(configValue); statErr == nil {
277215
err = appendCAFromFile(configValue, certsPool2)
278216
} else {
279217
err = appendCAFromContent(configValue, certsPool2)

0 commit comments

Comments
 (0)