Forward SSH known-hosts and InsecureSkipTLSverify to fleet apply

Pass FLEET_KNOWN_HOSTS and FLEET_INSECURE_SKIP_TLS_VERIFY through to
the bundlereader.Auth so that fleet apply CLI and the GitOps reconciler
honour the same TLS and SSH host-key settings as the rest of the
pipeline. Default the SSH username to 'git' when the URL has no user
component, matching the controller behaviour.

Add tests for createAuthFromOpts and addAuthToOpts, and update the
gitjob test to assert that InsecureSkipTLSverify is forwarded.

Author: thardeck

internal/cmd/cli/apply.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"fmt"
 	"os"
-	"strings"
 
 	gogit "github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
@@ -134,13 +133,6 @@ func (a *Apply) run(cmd *cobra.Command, args []string) error {
 		ImagescanEnabled:             a.ImagescanEnabled,
 	}
 
-	knownHostsPath, err := writeTmpKnownHosts()
-	if err != nil {
-		return err
-	}
-
-	defer os.RemoveAll(knownHostsPath)
-
 	if err := a.addAuthToOpts(&opts, os.ReadFile, a.HelmBasicHTTP, a.HelmInsecureSkipTLS); err != nil {
 		return fmt.Errorf("adding auth to opts: %w", err)
 	}
@@ -170,13 +162,6 @@ func (a *Apply) run(cmd *cobra.Command, args []string) error {
 		args = args[1:]
 	}
 
-	restoreEnv, err := setEnv(knownHostsPath)
-	if err != nil {
-		return fmt.Errorf("setting git SSH command env var for known hosts: %w", err)
-	}
-
-	defer restoreEnv() //nolint: errcheck // best-effort
-
 	ctx := cmd.Context()
 	cfg := ctrl.GetConfigOrDie()
 	client, err := client.New(cfg, client.Options{Scheme: scheme})
@@ -240,6 +225,9 @@ func (a *Apply) addAuthToOpts(opts *apply.Options, readFile readFile, helmBasicH
 
 	opts.Auth.BasicHTTP = helmBasicHTTP
 	opts.Auth.InsecureSkipVerify = helmInsecureSkipTLS
+	if raw := os.Getenv(ssh.KnownHostsEnvVar); raw != "" {
+		opts.Auth.SSHKnownHosts = []byte(raw)
+	}
 
 	return nil
 }
@@ -261,91 +249,6 @@ func currentCommit(dir string) string {
 // writeTmpKnownHosts creates a temporary file and writes known_hosts data to it, if such data is available from
 // environment variable `FLEET_KNOWN_HOSTS`.
 // It returns the name of the file and any error which may have happened while creating the file or writing to it.
-func writeTmpKnownHosts() (string, error) {
-	knownHosts, isSet := os.LookupEnv(ssh.KnownHostsEnvVar)
-	if !isSet || knownHosts == "" {
-		return "", nil
-	}
-
-	f, err := os.CreateTemp("", "known_hosts")
-	if err != nil {
-		return "", err
-	}
-
-	knownHostsPath := f.Name()
-
-	if err := os.WriteFile(knownHostsPath, []byte(knownHosts), 0600); err != nil {
-		return "", fmt.Errorf(
-			"failed to write value of %q env var to known_hosts file %s: %w",
-			ssh.KnownHostsEnvVar,
-			knownHostsPath,
-			err,
-		)
-	}
-
-	return knownHostsPath, nil
-}
-
-// setEnv sets the `GIT_SSH_COMMAND` environment variable with a known_hosts flag pointing to the provided
-// knownHostsPath. It takes care of preserving existing flags in the existing value of the environment variable, if any,
-// except for other user known_hosts file flags.
-// It returns a function to restore the environment variable to its initial value, and any error that might have
-// occurred in the process.
-func setEnv(knownHostsPath string) (func() error, error) {
-	commandEnvVar := "GIT_SSH_COMMAND"
-	flagName := "UserKnownHostsFile"
-
-	initialCommand, isSet := os.LookupEnv(commandEnvVar)
-
-	fail := func(err error) (func() error, error) {
-		return func() error { return nil }, err
-	}
-
-	if !isSet {
-		if err := os.Setenv(commandEnvVar, fmt.Sprintf("ssh -o %s=%s", flagName, knownHostsPath)); err != nil {
-			return fail(err)
-		}
-
-		return func() error { return os.Unsetenv(commandEnvVar) }, nil
-	}
-
-	// Check if `UserKnownHostsFile` is already present (case-insensitive), even multiple times, and skip it if so.
-	var newSSHCommand strings.Builder
-	options := strings.Split(initialCommand, " -o ")
-	for _, opt := range options {
-		kv := strings.Split(opt, "=")
-		if len(kv) != 2 { // first element, pre `-o`, or other flag
-			if _, err := newSSHCommand.WriteString(opt); err != nil {
-				return fail(err)
-			}
-
-			continue
-		}
-
-		if strings.EqualFold(kv[0], flagName) { // case-insensitive comparison
-			continue
-		}
-
-		if _, err := fmt.Fprintf(&newSSHCommand, " -o %s", opt); err != nil {
-			return fail(err)
-		}
-	}
-
-	if _, err := fmt.Fprintf(&newSSHCommand, " -o %s=%s", flagName, knownHostsPath); err != nil {
-		return fail(err)
-	}
-
-	if err := os.Setenv(commandEnvVar, newSSHCommand.String()); err != nil {
-		return fail(err)
-	}
-
-	restore := func() error {
-		return os.Setenv(commandEnvVar, initialCommand)
-	}
-
-	return restore, nil
-}
-
 func getEventRecorder(config *rest.Config, componentName string) (record.EventRecorder, error) {
 	clientset, err := kubernetes.NewForConfig(config)
 	if err != nil {

internal/cmd/cli/apply_test.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/rancher/fleet/internal/bundlereader"
 	"github.com/rancher/fleet/internal/cmd/cli/apply"
+	ssh "github.com/rancher/fleet/internal/ssh"
 )
 
 const (
@@ -34,146 +35,25 @@ const (
 
 var helmSecretsNameByPath_content = map[string]bundlereader.Auth{"path": {Username: username, Password: password_content}}
 
-func TestSetEnv(t *testing.T) {
-	tests := map[string]struct {
-		envValue              string
-		knownHostsPath        string
-		expectedGitSSHCommand string
-		expectedErr           error
-	}{
-		"unset env var": {
-			knownHostsPath:        "/foo/bar",
-			expectedGitSSHCommand: "ssh -o UserKnownHostsFile=/foo/bar",
-		},
-		"set env var without options": {
-			envValue:              "ssh",
-			knownHostsPath:        "/foo/bar",
-			expectedGitSSHCommand: "ssh -o UserKnownHostsFile=/foo/bar",
-		},
-		"set env var with other options": {
-			envValue:              "ssh -o stricthostkeychecking=yes",
-			knownHostsPath:        "/foo/bar",
-			expectedGitSSHCommand: "ssh -o stricthostkeychecking=yes -o UserKnownHostsFile=/foo/bar",
-		},
-		"set env var with other options and known hosts file option": {
-			envValue:              "ssh -o stricthostkeychecking=yes -o userknownhostsFile=/another/file",
-			knownHostsPath:        "/foo/bar",
-			expectedGitSSHCommand: "ssh -o stricthostkeychecking=yes -o UserKnownHostsFile=/foo/bar",
-		},
-		"set env var with other options and known hosts file option specified multiple times": {
-			envValue:              "ssh -o userknownhostsFile=/another/file -o UserKnownHostsFile=/yet/another/file -o stricthostkeychecking=yes",
-			knownHostsPath:        "/foo/bar",
-			expectedGitSSHCommand: "ssh -o stricthostkeychecking=yes -o UserKnownHostsFile=/foo/bar",
-		},
-	}
-
-	bkpEnv := os.Getenv("GIT_SSH_COMMAND")
-	defer os.Setenv("GIT_SSH_COMMAND", bkpEnv)
-
-	for name, test := range tests {
-		t.Run(name, func(t *testing.T) {
-			defer os.Unsetenv("GIT_SSH_COMMAND")
-
-			if test.envValue != "" {
-				os.Setenv("GIT_SSH_COMMAND", test.envValue)
-			} else {
-				os.Unsetenv("GIT_SSH_COMMAND")
-			}
-
-			restore, err := setEnv(test.knownHostsPath)
-			if !errors.Is(err, test.expectedErr) {
-				t.Errorf("expected err %v, got %v", test.expectedErr, err)
-			}
-
-			if gitSSHCommand := os.Getenv("GIT_SSH_COMMAND"); gitSSHCommand != test.expectedGitSSHCommand {
-				t.Errorf("expected GIT_SSH_COMMAND %q, got %q", test.expectedGitSSHCommand, gitSSHCommand)
-			}
-
-			if restoreErr := restore(); restoreErr != nil {
-				t.Errorf("expected nil restore error, got %v", restoreErr)
-			}
-
-			restoredEnvValue, isSet := os.LookupEnv("GIT_SSH_COMMAND")
-			if restoredEnvValue != test.envValue {
-				t.Errorf(
-					"expected restored GIT_SSH_COMMAND value to be %q, got %t/%q",
-					test.envValue,
-					isSet,
-					restoredEnvValue,
-				)
-			}
-		})
-	}
-}
-
-func TestWriteTmpKnownHosts(t *testing.T) {
-	tests := map[string]struct {
-		knownHosts       string
-		isSet            bool
-		expectFileExists bool
-	}{
-		"does not write to known hosts file if FLEET_KNOWN_HOSTS is unset": {},
-		"does not write to known hosts file if FLEET_KNOWN_HOSTS is empty": {isSet: true},
-		"writes FLEET_KNOWN_HOSTS to custom known hosts file if set": {
-			knownHosts:       "foo",
-			isSet:            true,
-			expectFileExists: true,
-		},
-	}
-
-	for name, test := range tests {
-		t.Run(name, func(t *testing.T) {
-			if test.isSet {
-				if err := os.Setenv("FLEET_KNOWN_HOSTS", test.knownHosts); err != nil {
-					t.Errorf("failed to set FLEET_KNOWN_HOSTS env var: %v", err)
-				}
-
-				defer os.Unsetenv("FLEET_KNOWN_HOSTS")
-			}
-
-			khPath, err := writeTmpKnownHosts()
-			if err != nil {
-				t.Errorf("expected nil error from writeTmpKnownHosts, got: %v", err)
-			}
-
-			if !test.expectFileExists {
-				return
-			}
-
-			gotKnownHosts, err := os.ReadFile(khPath)
-			if err != nil {
-				t.Errorf("failed to read known_hosts file: %v", err)
-			}
-
-			defer os.RemoveAll(khPath)
-
-			if test.knownHosts != "" {
-				if string(gotKnownHosts) != test.knownHosts {
-					t.Errorf("known_hosts mismatch: expected\n\t%s\ngot:\n\t%s", test.knownHosts, gotKnownHosts)
-				}
-			}
-		})
-	}
-}
-
 func TestAddAuthToOpts(t *testing.T) {
 	tests := map[string]struct {
-		name         string
-		apply        Apply
-		knownHosts   string
-		expectedOpts *apply.Options
-		expectedErr  error
+		name                string
+		apply               Apply
+		knownHosts          string
+		helmInsecureSkipTLS bool
+		expectedOpts        *apply.Options
+		expectedErr         error
 	}{
 		"Auth is empty if no arguments are provided": {
 			apply:        Apply{},
 			expectedOpts: &apply.Options{},
 			expectedErr:  nil,
 		},
-		"known_hosts file is populated if the env var is set": {
+		"FLEET_KNOWN_HOSTS env var sets SSHKnownHosts in opts": {
 			apply:        Apply{},
-			expectedOpts: &apply.Options{},
+			knownHosts:   "some-known-host",
+			expectedOpts: &apply.Options{Auth: bundlereader.Auth{SSHKnownHosts: []byte("some-known-host")}},
 			expectedErr:  nil,
-			knownHosts:   "foo",
 		},
 		"Auth contains values from username, password, caCerts and sshPrivatey when helmSecretsNameByPath not provided": {
 			apply:        Apply{PasswordFile: password_file, Username: username, CACertsFile: caCerts_file, SSHPrivateKeyFile: sshPrivateKey_file},
@@ -190,6 +70,12 @@ func TestAddAuthToOpts(t *testing.T) {
 			expectedOpts: &apply.Options{AuthByPath: helmSecretsNameByPath_content},
 			expectedErr:  nil,
 		},
+		"HelmInsecureSkipTLS sets InsecureSkipVerify in opts": {
+			apply:               Apply{},
+			helmInsecureSkipTLS: true,
+			expectedOpts:        &apply.Options{Auth: bundlereader.Auth{InsecureSkipVerify: true}},
+			expectedErr:         nil,
+		},
 		"Error if file doesn't exist": {
 			apply:        Apply{HelmCredentialsByPathFile: "notfound"},
 			expectedOpts: &apply.Options{},
@@ -199,8 +85,11 @@ func TestAddAuthToOpts(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
+			if test.knownHosts != "" {
+				t.Setenv(ssh.KnownHostsEnvVar, test.knownHosts)
+			}
 			opts := &apply.Options{}
-			err := test.apply.addAuthToOpts(opts, mockReadFile, false, false)
+			err := test.apply.addAuthToOpts(opts, mockReadFile, false, test.helmInsecureSkipTLS)
 			if !cmp.Equal(opts, test.expectedOpts) {
 				t.Errorf("opts don't match: expected %v, got %v", test.expectedOpts, opts)
 			}