diff --git a/e2e/single-cluster/oci_registry_test.go b/e2e/single-cluster/oci_registry_test.go index 4020043802..2e82b8a22d 100644 --- a/e2e/single-cluster/oci_registry_test.go +++ b/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), } diff --git a/internal/cmd/cli/apply/apply.go b/internal/cmd/cli/apply/apply.go index 4992e08256..78b6e1e17e 100644 --- a/internal/cmd/cli/apply/apply.go +++ b/internal/cmd/cli/apply/apply.go @@ -758,13 +758,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, } diff --git a/internal/cmd/cli/apply/apply_test.go b/internal/cmd/cli/apply/apply_test.go index a25001eea6..ed6c76b64b 100644 --- a/internal/cmd/cli/apply/apply_test.go +++ b/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) { @@ -87,3 +90,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") + } +} diff --git a/internal/ocistorage/secret.go b/internal/ocistorage/secret.go index 21ed198ade..d495edb9cb 100644 --- a/internal/ocistorage/secret.go +++ b/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]) +} diff --git a/internal/ocistorage/secret_test.go b/internal/ocistorage/secret_test.go index 34c61e757f..8df54e472b 100644 --- a/internal/ocistorage/secret_test.go +++ b/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 = ""