Skip to content

Commit 8fbd448

Browse files
khushalchandak17weyfonk
khushalchandak17
and
weyfonk
committed
fix: honor insecureSkipTLS in OCI storage secret (rancher#4890)
Fleet currently reads the legacy `insecure` field from the OCI storage secret. If the secret is created with `insecureSkipTLS: true`, that value is not recognized, so Fleet agents still perform TLS verification and may fail against registries using a self-signed/private CA. This change: - prefers `insecureSkipTLS` - keeps `insecure` as a backward-compatible fallback - updates test coverage to use `insecureSkipTLS` This is intended to be a minimal fix for OCI storage secret parsing and avoids breaking existing setups that may still use the legacy `insecure` field. Co-authored-by: Corentin Néau <tan.neau@suse.com> (cherry picked from commit 3951853)
1 parent 3c79f6f commit 8fbd448

5 files changed

Lines changed: 201 additions & 22 deletions

File tree

‎e2e/single-cluster/oci_registry_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,13 @@ func createOCIRegistrySecret(
5454
Namespace: namespace,
5555
},
5656
Data: map[string][]byte{
57-
ocistorage.OCISecretReference: []byte(reference),
58-
ocistorage.OCISecretUsername: []byte(username),
59-
ocistorage.OCISecretPassword: []byte(password),
60-
ocistorage.OCISecretAgentUsername: []byte(agentUsername),
61-
ocistorage.OCISecretAgentPassword: []byte(agentPassword),
62-
ocistorage.OCISecretInsecure: []byte(strconv.FormatBool(insecure)),
63-
ocistorage.OCISecretBasicHTTP: []byte(strconv.FormatBool(false)),
57+
ocistorage.OCISecretReference: []byte(reference),
58+
ocistorage.OCISecretUsername: []byte(username),
59+
ocistorage.OCISecretPassword: []byte(password),
60+
ocistorage.OCISecretAgentUsername: []byte(agentUsername),
61+
ocistorage.OCISecretAgentPassword: []byte(agentPassword),
62+
ocistorage.OCISecretInsecureSkipTLS: []byte(strconv.FormatBool(insecure)),
63+
ocistorage.OCISecretBasicHTTP: []byte(strconv.FormatBool(false)),
6464
},
6565
Type: corev1.SecretType(fleet.SecretTypeOCIStorage),
6666
}

‎internal/cmd/cli/apply/apply.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -674,13 +674,13 @@ func newOCISecret(manifestID string, bundle *fleet.Bundle, opts ocistorage.OCIOp
674674
},
675675
},
676676
Data: map[string][]byte{
677-
ocistorage.OCISecretReference: []byte(opts.Reference),
678-
ocistorage.OCISecretUsername: []byte(opts.Username),
679-
ocistorage.OCISecretPassword: []byte(opts.Password),
680-
ocistorage.OCISecretAgentUsername: []byte(opts.AgentUsername),
681-
ocistorage.OCISecretAgentPassword: []byte(opts.AgentPassword),
682-
ocistorage.OCISecretBasicHTTP: []byte(strconv.FormatBool(opts.BasicHTTP)),
683-
ocistorage.OCISecretInsecure: []byte(strconv.FormatBool(opts.InsecureSkipTLS)),
677+
ocistorage.OCISecretReference: []byte(opts.Reference),
678+
ocistorage.OCISecretUsername: []byte(opts.Username),
679+
ocistorage.OCISecretPassword: []byte(opts.Password),
680+
ocistorage.OCISecretAgentUsername: []byte(opts.AgentUsername),
681+
ocistorage.OCISecretAgentPassword: []byte(opts.AgentPassword),
682+
ocistorage.OCISecretBasicHTTP: []byte(strconv.FormatBool(opts.BasicHTTP)),
683+
ocistorage.OCISecretInsecureSkipTLS: []byte(strconv.FormatBool(opts.InsecureSkipTLS)),
684684
},
685685
Type: fleet.SecretTypeOCIStorage,
686686
}

‎internal/cmd/cli/apply/apply_test.go‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
package apply
2+
3+
import (
4+
"testing"
5+
6+
"github.com/rancher/fleet/internal/ocistorage"
7+
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"k8s.io/apimachinery/pkg/types"
10+
)
11+
12+
func Test_getKindNS(t *testing.T) {
13+
cases := []struct {
14+
name string
15+
input fleet.BundleResource
16+
expectedOutput fleet.OverwrittenResource
17+
}{
18+
{
19+
name: "templated contents",
20+
input: fleet.BundleResource{
21+
Name: "folder/my-cm.yaml",
22+
Content: `apiVersion: v1
23+
kind: ConfigMap
24+
metadata:
25+
name: Foo
26+
namespace: my-namespace
27+
data:
28+
bar: baz
29+
name: {{ .Values.name }}`,
30+
// No encoding
31+
},
32+
expectedOutput: fleet.OverwrittenResource{
33+
Kind: "ConfigMap",
34+
Name: "Foo",
35+
Namespace: "my-namespace",
36+
},
37+
},
38+
{
39+
name: "templated name",
40+
input: fleet.BundleResource{
41+
Name: "folder/my-cm.yaml",
42+
Content: `apiVersion: v1
43+
kind: ConfigMap
44+
metadata:
45+
name: {{ .Values.name }}
46+
namespace: my-namespace
47+
data:
48+
bar: baz
49+
name: Foo`,
50+
// No encoding
51+
},
52+
expectedOutput: fleet.OverwrittenResource{
53+
Kind: "ConfigMap",
54+
Name: "TEMPLATED",
55+
Namespace: "my-namespace",
56+
},
57+
},
58+
{
59+
name: "templated namespace",
60+
input: fleet.BundleResource{
61+
Name: "folder/my-cm.yaml",
62+
Content: `apiVersion: v1
63+
kind: ConfigMap
64+
metadata:
65+
name: Foo
66+
namespace: {{ .Values.namespace }}
67+
data:
68+
bar: baz
69+
name: Foo`,
70+
// No encoding
71+
},
72+
expectedOutput: fleet.OverwrittenResource{
73+
Kind: "ConfigMap",
74+
Name: "Foo",
75+
Namespace: "TEMPLATED",
76+
},
77+
},
78+
}
79+
80+
for _, tc := range cases {
81+
t.Run(tc.name, func(t *testing.T) {
82+
or, err := getKindNS(tc.input, "my-bundle")
83+
if err != nil {
84+
t.Fatalf("expected no error, got %v", err)
85+
}
86+
87+
if or != tc.expectedOutput {
88+
t.Fatalf("expected OverwrittenResource\n\t%v, got\n\t%v", tc.expectedOutput, or)
89+
}
90+
})
91+
}
92+
}
93+
94+
func Test_newOCISecret_usesInsecureSkipTLSKey(t *testing.T) {
95+
bundle := &fleet.Bundle{
96+
ObjectMeta: metav1.ObjectMeta{
97+
Name: "bundle",
98+
Namespace: "fleet-local",
99+
UID: types.UID("bundle-uid"),
100+
},
101+
}
102+
103+
secret := newOCISecret("manifest-id", bundle, ocistorage.OCIOpts{
104+
Reference: "registry.example.com/test",
105+
Username: "user",
106+
Password: "pass",
107+
AgentUsername: "agent-user",
108+
AgentPassword: "agent-pass",
109+
InsecureSkipTLS: true,
110+
})
111+
112+
if got := string(secret.Data[ocistorage.OCISecretInsecureSkipTLS]); got != "true" {
113+
t.Fatalf("expected insecureSkipTLS=true, got %q", got)
114+
}
115+
116+
if _, ok := secret.Data[ocistorage.OCISecretInsecure]; ok {
117+
t.Fatal("did not expect legacy insecure key in generated secret")
118+
}
119+
}

‎internal/ocistorage/secret.go‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@ import (
1313
)
1414

1515
const (
16-
OCISecretUsername = "username"
17-
OCISecretPassword = "password"
18-
OCISecretAgentUsername = "agentUsername"
19-
OCISecretAgentPassword = "agentPassword"
20-
OCISecretReference = "reference"
21-
OCISecretBasicHTTP = "basicHTTP"
22-
OCISecretInsecure = "insecure"
16+
OCISecretUsername = "username"
17+
OCISecretPassword = "password"
18+
OCISecretAgentUsername = "agentUsername"
19+
OCISecretAgentPassword = "agentPassword"
20+
OCISecretReference = "reference"
21+
OCISecretBasicHTTP = "basicHTTP"
22+
OCISecretInsecureSkipTLS = "insecureSkipTLS"
23+
OCISecretInsecure = "insecure" // legacy alias
2324
)
2425

2526
// ReadOptsFromSecret reads the secret identified by the given NamespacedName and
@@ -73,7 +74,12 @@ func ReadOptsFromSecret(ctx context.Context, c client.Reader, ns client.ObjectKe
7374
return OCIOpts{}, err
7475
}
7576

76-
opts.InsecureSkipTLS, err = getBoolValueFromSecret(secret.Data, OCISecretInsecure, false)
77+
opts.InsecureSkipTLS, err = getBoolValueFromSecretWithFallback(
78+
secret.Data,
79+
false,
80+
OCISecretInsecureSkipTLS,
81+
OCISecretInsecure,
82+
)
7783
if err != nil {
7884
return OCIOpts{}, err
7985
}
@@ -108,3 +114,17 @@ func getBoolValueFromSecret(data map[string][]byte, key string, required bool) (
108114

109115
return boolValue, nil
110116
}
117+
118+
// getBoolValueFromSecretWithFallback extracts a boolean value from data, using keys in the provided order of priority, and returns the first found value, if any.
119+
// If no value is found, the function returns false, with an error if the value was required.
120+
func getBoolValueFromSecretWithFallback(data map[string][]byte, required bool, keys ...string) (bool, error) {
121+
for _, key := range keys {
122+
if _, ok := data[key]; ok {
123+
return getBoolValueFromSecret(data, key, true)
124+
}
125+
}
126+
if !required {
127+
return false, nil
128+
}
129+
return false, fmt.Errorf("key %q not found in secret", keys[0])
130+
}

‎internal/ocistorage/secret_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,46 @@ var _ = Describe("OCIOpts loaded from secret", func() {
7676
})
7777
})
7878

79+
When("the given oci storage secret uses the documented insecureSkipTLS field", func() {
80+
BeforeEach(func() {
81+
secretName = "test"
82+
secretData = map[string][]byte{
83+
OCISecretReference: []byte("reference"),
84+
OCISecretInsecureSkipTLS: []byte("true"),
85+
}
86+
secretType = fleet.SecretTypeOCIStorage
87+
secretGetErrorMessage = ""
88+
secretGetNotFoundError = false
89+
})
90+
It("returns the expected OCIOpts from the data in the secret", func() {
91+
ns := client.ObjectKey{Name: secretName, Namespace: "test"}
92+
opts, err := ReadOptsFromSecret(context.TODO(), mockClient, ns)
93+
Expect(err).ToNot(HaveOccurred())
94+
Expect(opts.Reference).To(Equal(string(secretData[OCISecretReference])))
95+
Expect(opts.InsecureSkipTLS).To(BeTrue())
96+
})
97+
})
98+
99+
When("the oci storage secret contains both insecure keys", func() {
100+
BeforeEach(func() {
101+
secretName = "test"
102+
secretData = map[string][]byte{
103+
OCISecretReference: []byte("reference"),
104+
OCISecretInsecureSkipTLS: []byte("false"),
105+
OCISecretInsecure: []byte("true"),
106+
}
107+
secretType = fleet.SecretTypeOCIStorage
108+
secretGetErrorMessage = ""
109+
secretGetNotFoundError = false
110+
})
111+
It("prefers insecureSkipTLS over the legacy insecure field", func() {
112+
ns := client.ObjectKey{Name: secretName, Namespace: "test"}
113+
opts, err := ReadOptsFromSecret(context.TODO(), mockClient, ns)
114+
Expect(err).ToNot(HaveOccurred())
115+
Expect(opts.InsecureSkipTLS).To(BeFalse())
116+
})
117+
})
118+
79119
When("the secret name is not set, but a default secret exists", func() {
80120
BeforeEach(func() {
81121
secretName = ""

0 commit comments

Comments
 (0)