fix: honor insecureSkipTLS in OCI storage secret (#4890) (#4926)

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.


(cherry picked from commit 3951853)

Co-authored-by: Corentin Néau <tan.neau@suse.com>

Author: khushalchandak17

e2e/single-cluster/oci_registry_test.go

@@ -54,13 +54,13 @@ func createOCIRegistrySecret(
 			Namespace: namespace,
 		},
 		Data: map[string][]byte{
-			ocistorage.OCISecretReference:     []byte(reference),
-			ocistorage.OCISecretUsername:      []byte(username),
-			ocistorage.OCISecretPassword:      []byte(password),
-			ocistorage.OCISecretAgentUsername: []byte(agentUsername),
-			ocistorage.OCISecretAgentPassword: []byte(agentPassword),
-			ocistorage.OCISecretInsecure:      []byte(strconv.FormatBool(insecure)),
-			ocistorage.OCISecretBasicHTTP:     []byte(strconv.FormatBool(false)),
+			ocistorage.OCISecretReference:       []byte(reference),
+			ocistorage.OCISecretUsername:        []byte(username),
+			ocistorage.OCISecretPassword:        []byte(password),
+			ocistorage.OCISecretAgentUsername:   []byte(agentUsername),
+			ocistorage.OCISecretAgentPassword:   []byte(agentPassword),
+			ocistorage.OCISecretInsecureSkipTLS: []byte(strconv.FormatBool(insecure)),
+			ocistorage.OCISecretBasicHTTP:       []byte(strconv.FormatBool(false)),
 		},
 		Type: corev1.SecretType(fleet.SecretTypeOCIStorage),
 	}

internal/cmd/cli/apply/apply.go

@@ -757,13 +757,13 @@ func newOCISecret(manifestID string, bundle *fleet.Bundle, opts ocistorage.OCIOp
 			},
 		},
 		Data: map[string][]byte{
-			ocistorage.OCISecretReference:     []byte(opts.Reference),
-			ocistorage.OCISecretUsername:      []byte(opts.Username),
-			ocistorage.OCISecretPassword:      []byte(opts.Password),
-			ocistorage.OCISecretAgentUsername: []byte(opts.AgentUsername),
-			ocistorage.OCISecretAgentPassword: []byte(opts.AgentPassword),
-			ocistorage.OCISecretBasicHTTP:     []byte(strconv.FormatBool(opts.BasicHTTP)),
-			ocistorage.OCISecretInsecure:      []byte(strconv.FormatBool(opts.InsecureSkipTLS)),
+			ocistorage.OCISecretReference:       []byte(opts.Reference),
+			ocistorage.OCISecretUsername:        []byte(opts.Username),
+			ocistorage.OCISecretPassword:        []byte(opts.Password),
+			ocistorage.OCISecretAgentUsername:   []byte(opts.AgentUsername),
+			ocistorage.OCISecretAgentPassword:   []byte(opts.AgentPassword),
+			ocistorage.OCISecretBasicHTTP:       []byte(strconv.FormatBool(opts.BasicHTTP)),
+			ocistorage.OCISecretInsecureSkipTLS: []byte(strconv.FormatBool(opts.InsecureSkipTLS)),
 		},
 		Type: fleet.SecretTypeOCIStorage,
 	}

internal/cmd/cli/apply/apply_test.go

@@ -3,7 +3,10 @@ package apply
 import (
 	"testing"
 
+	"github.com/rancher/fleet/internal/ocistorage"
 	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 func Test_getKindNS(t *testing.T) {
@@ -40,3 +43,30 @@ data:
 	}
 
 }
+
+func Test_newOCISecret_usesInsecureSkipTLSKey(t *testing.T) {
+	bundle := &fleet.Bundle{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "bundle",
+			Namespace: "fleet-local",
+			UID:       types.UID("bundle-uid"),
+		},
+	}
+
+	secret := newOCISecret("manifest-id", bundle, ocistorage.OCIOpts{
+		Reference:       "registry.example.com/test",
+		Username:        "user",
+		Password:        "pass",
+		AgentUsername:   "agent-user",
+		AgentPassword:   "agent-pass",
+		InsecureSkipTLS: true,
+	})
+
+	if got := string(secret.Data[ocistorage.OCISecretInsecureSkipTLS]); got != "true" {
+		t.Fatalf("expected insecureSkipTLS=true, got %q", got)
+	}
+
+	if _, ok := secret.Data[ocistorage.OCISecretInsecure]; ok {
+		t.Fatal("did not expect legacy insecure key in generated secret")
+	}
+}

internal/ocistorage/secret.go

@@ -13,13 +13,14 @@ import (
 )
 
 const (
-	OCISecretUsername      = "username"
-	OCISecretPassword      = "password"
-	OCISecretAgentUsername = "agentUsername"
-	OCISecretAgentPassword = "agentPassword"
-	OCISecretReference     = "reference"
-	OCISecretBasicHTTP     = "basicHTTP"
-	OCISecretInsecure      = "insecure"
+	OCISecretUsername        = "username"
+	OCISecretPassword        = "password"
+	OCISecretAgentUsername   = "agentUsername"
+	OCISecretAgentPassword   = "agentPassword"
+	OCISecretReference       = "reference"
+	OCISecretBasicHTTP       = "basicHTTP"
+	OCISecretInsecureSkipTLS = "insecureSkipTLS"
+	OCISecretInsecure        = "insecure" // legacy alias
 )
 
 // 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
 		return OCIOpts{}, err
 	}
 
-	opts.InsecureSkipTLS, err = getBoolValueFromSecret(secret.Data, OCISecretInsecure, false)
+	opts.InsecureSkipTLS, err = getBoolValueFromSecretWithFallback(
+		secret.Data,
+		false,
+		OCISecretInsecureSkipTLS,
+		OCISecretInsecure,
+	)
 	if err != nil {
 		return OCIOpts{}, err
 	}
@@ -108,3 +114,17 @@ func getBoolValueFromSecret(data map[string][]byte, key string, required bool) (
 
 	return boolValue, nil
 }
+
+// getBoolValueFromSecretWithFallback extracts a boolean value from data, using keys in the provided order of priority, and returns the first found value, if any.
+// If no value is found, the function returns false, with an error if the value was required.
+func getBoolValueFromSecretWithFallback(data map[string][]byte, required bool, keys ...string) (bool, error) {
+	for _, key := range keys {
+		if _, ok := data[key]; ok {
+			return getBoolValueFromSecret(data, key, true)
+		}
+	}
+	if !required {
+		return false, nil
+	}
+	return false, fmt.Errorf("key %q not found in secret", keys[0])
+}

internal/ocistorage/secret_test.go

@@ -76,6 +76,46 @@ var _ = Describe("OCIOpts loaded from secret", func() {
 		})
 	})
 
+	When("the given oci storage secret uses the documented insecureSkipTLS field", func() {
+		BeforeEach(func() {
+			secretName = "test"
+			secretData = map[string][]byte{
+				OCISecretReference:       []byte("reference"),
+				OCISecretInsecureSkipTLS: []byte("true"),
+			}
+			secretType = fleet.SecretTypeOCIStorage
+			secretGetErrorMessage = ""
+			secretGetNotFoundError = false
+		})
+		It("returns the expected OCIOpts from the data in the secret", func() {
+			ns := client.ObjectKey{Name: secretName, Namespace: "test"}
+			opts, err := ReadOptsFromSecret(context.TODO(), mockClient, ns)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(opts.Reference).To(Equal(string(secretData[OCISecretReference])))
+			Expect(opts.InsecureSkipTLS).To(BeTrue())
+		})
+	})
+
+	When("the oci storage secret contains both insecure keys", func() {
+		BeforeEach(func() {
+			secretName = "test"
+			secretData = map[string][]byte{
+				OCISecretReference:       []byte("reference"),
+				OCISecretInsecureSkipTLS: []byte("false"),
+				OCISecretInsecure:        []byte("true"),
+			}
+			secretType = fleet.SecretTypeOCIStorage
+			secretGetErrorMessage = ""
+			secretGetNotFoundError = false
+		})
+		It("prefers insecureSkipTLS over the legacy insecure field", func() {
+			ns := client.ObjectKey{Name: secretName, Namespace: "test"}
+			opts, err := ReadOptsFromSecret(context.TODO(), mockClient, ns)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(opts.InsecureSkipTLS).To(BeFalse())
+		})
+	})
+
 	When("the secret name is not set, but a default secret exists", func() {
 		BeforeEach(func() {
 			secretName = ""