cleanup: Inject a single k8s.Client instead of reading the kubeconfig ad hoc

cmd/build.go

@@ -15,6 +15,7 @@ import (
 	"knative.dev/func/pkg/config"
 	"knative.dev/func/pkg/docker"
 	fn "knative.dev/func/pkg/functions"
+	"knative.dev/func/pkg/k8s"
 	"knative.dev/func/pkg/oci"
 	"knative.dev/func/pkg/s2i"
 )
@@ -171,12 +172,13 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 
 	f = cfg.Configure(f) // Returns an f updated with values from the config (flags, envs, etc)
 
-	// Client
-	clientOptions, err := cfg.clientOptions()
+	// Kube Client
+	kc := k8s.NewClientFromKubeconfig()
+	clientOptions, err := cfg.clientOptions(kc)
 	if err != nil {
 		return
 	}
-	client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, clientOptions...)
+	client, done := newClient(ClientConfig{Verbose: cfg.Verbose, K8sClient: kc}, clientOptions...)
 	defer done()
 
 	// Build
@@ -441,22 +443,22 @@ func (c buildConfig) Validate(cmd *cobra.Command) (err error) {
 // TODO: As a further optimization, it might be ideal to only build the
 // image necessary for the target cluster, since the end product of a function
 // deployment is not the container, but rather the running service.
-func (c buildConfig) clientOptions() ([]fn.Option, error) {
+func (c buildConfig) clientOptions(kc *k8s.Client) ([]fn.Option, error) {
 	o := []fn.Option{
 		fn.WithRegistry(c.Registry),
 		fn.WithRegistryInsecure(c.RegistryInsecure),
 	}
 
-	t := newTransport(c.RegistryInsecure)
-	creds := newCredentialsProvider(config.Dir(), t, c.RegistryAuthfile, c.RegistryInsecure)
+	t := newTransport(kc, c.RegistryInsecure)
+	creds := newCredentialsProvider(kc, config.Dir(), t, c.RegistryAuthfile, c.RegistryInsecure)
 
 	switch c.Builder {
 	case builders.Host:
 		o = append(o,
 			fn.WithScaffolder(oci.NewScaffolder(c.Verbose)),
 			fn.WithBuilder(oci.NewBuilder(builders.Host, c.Verbose)),
 			fn.WithPusher(oci.NewPusher(c.RegistryInsecure, false, c.Verbose,
-				oci.WithTransport(newTransport(c.RegistryInsecure)),
+				oci.WithTransport(newTransport(kc, c.RegistryInsecure)),
 				oci.WithCredentialsProvider(creds),
 				oci.WithVerbose(c.Verbose))),
 		)

cmd/client.go

@@ -34,6 +34,11 @@ type ClientConfig struct {
 
 	// Allow insecure server connections when using SSL
 	InsecureSkipVerify bool
+
+	// K8sClient is the cluster client every cluster-facing component uses.
+	// Commands resolve it once and pass it here. If nil, NewClient resolves
+	// it from the kubeconfig.
+	K8sClient *k8s.Client
 }
 
 // ClientFactory defines a constructor which assists in the creation of a Client
@@ -62,23 +67,23 @@ func NewTestClient(options ...fn.Option) ClientFactory {
 // 'Verbose' indicates the system should write out a higher amount of logging.
 func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) {
 	var (
-		kc = k8s.NewClient(k8s.GetClientConfig())
-		t  = newTransport(cfg.InsecureSkipVerify)                                // may provide a custom impl which proxies
-		c  = newCredentialsProvider(config.Dir(), t, "", cfg.InsecureSkipVerify) // for accessing registries
-		d  = newKnativeDeployer(cfg.Verbose)                                     // default deployer (can be overridden via options)
-		pp = newTektonPipelinesProvider(c, cfg.Verbose, t)
+		kc = newK8sClient(cfg.K8sClient)
+		t  = newTransport(kc, cfg.InsecureSkipVerify)                                // may provide a custom impl which proxies
+		c  = newCredentialsProvider(kc, config.Dir(), t, "", cfg.InsecureSkipVerify) // for accessing registries
+		d  = newKnativeDeployer(kc, cfg.Verbose)                                     // default deployer (can be overridden via options)
+		pp = newTektonPipelinesProvider(kc, c, cfg.Verbose, t)
 		o  = []fn.Option{ // standard (shared) options for all commands
 			fn.WithVerbose(cfg.Verbose),
 			fn.WithTransport(t),
 			fn.WithRepositoriesPath(config.RepositoriesPath()),
 			fn.WithScaffolder(buildpacks.NewScaffolder(cfg.Verbose)),
 			fn.WithBuilder(buildpacks.NewBuilder(buildpacks.WithVerbose(cfg.Verbose))),
-			fn.WithRemovers(knative.NewRemover(cfg.Verbose), k8s.NewRemover(cfg.Verbose),
-				keda.NewRemover(cfg.Verbose)),
+			fn.WithRemovers(knative.NewRemover(kc, cfg.Verbose), k8s.NewRemover(kc, cfg.Verbose),
+				keda.NewRemover(kc, cfg.Verbose)),
 			fn.WithDescribers(
-				knative.NewDescriber(cfg.Verbose, knative.WithDescriberTransport(t)),
-				k8s.NewDescriber(cfg.Verbose, k8s.WithDescriberTransport(t)),
-				keda.NewDescriber(cfg.Verbose, keda.WithDescriberTransport(t)),
+				knative.NewDescriber(kc, cfg.Verbose, knative.WithDescriberTransport(t)),
+				k8s.NewDescriber(kc, cfg.Verbose, k8s.WithDescriberTransport(t)),
+				keda.NewDescriber(kc, cfg.Verbose, keda.WithDescriberTransport(t)),
 			),
 			fn.WithListers(knative.NewLister(kc, cfg.Verbose), k8s.NewLister(kc, cfg.Verbose), keda.NewLister(kc, cfg.Verbose)),
 			fn.WithDeployer(d),
@@ -88,7 +93,7 @@ func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) {
 				docker.WithTransport(t),
 				docker.WithVerbose(cfg.Verbose),
 				docker.WithInsecure(cfg.InsecureSkipVerify))),
-			fn.WithSyncer(operator.NewSyncer(operator.WithCredentialsProvider(c))),
+			fn.WithSyncer(operator.NewSyncer(kc, operator.WithCredentialsProvider(c))),
 		}
 	)
 
@@ -107,19 +112,28 @@ func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) {
 	return client, cleanup
 }
 
+// newK8sClient returns kc, or a client resolved from the kubeconfig when kc
+// is nil. This is the one place a command falls back to the kubeconfig.
+func newK8sClient(kc *k8s.Client) *k8s.Client {
+	if kc != nil {
+		return kc
+	}
+	return k8s.NewClientFromKubeconfig()
+}
+
 // newTransport returns a transport with cluster-flavor-specific variations
 // which take advantage of additional features offered by cluster variants.
-func newTransport(insecureSkipVerify bool) fnhttp.RoundTripCloser {
-	return fnhttp.NewRoundTripper(fnhttp.WithInsecureSkipVerify(insecureSkipVerify), fnhttp.WithOpenShiftServiceCA())
+func newTransport(kc *k8s.Client, insecureSkipVerify bool) fnhttp.RoundTripCloser {
+	return fnhttp.NewRoundTripper(kc, fnhttp.WithInsecureSkipVerify(insecureSkipVerify), fnhttp.WithOpenShiftServiceCA(kc))
 }
 
 // newCredentialsProvider returns a credentials provider which possibly
 // has cluster-flavor specific additional credential loaders to take advantage
 // of features or configuration nuances of cluster variants.
 // If authFilePath is provided (non-empty), it will be used as the primary auth file.
 // When insecure is true, credential verification uses plain HTTP instead of HTTPS.
-func newCredentialsProvider(configPath string, t http.RoundTripper, authFilePath string, insecure bool) oci.CredentialsProvider {
-	additionalLoaders := append(k8s.GetOpenShiftDockerCredentialLoaders(), k8s.GetGoogleCredentialLoader()...)
+func newCredentialsProvider(kc *k8s.Client, configPath string, t http.RoundTripper, authFilePath string, insecure bool) oci.CredentialsProvider {
+	additionalLoaders := append(kc.OpenShiftDockerCredentialLoaders(), k8s.GetGoogleCredentialLoader()...)
 	additionalLoaders = append(additionalLoaders, k8s.GetECRCredentialLoader()...)
 	additionalLoaders = append(additionalLoaders, k8s.GetACRCredentialLoader()...)
 
@@ -156,35 +170,33 @@ func newCredentialsProvider(configPath string, t http.RoundTripper, authFilePath
 	return creds.NewCredentialsProvider(configPath, options...)
 }
 
-func newTektonPipelinesProvider(creds oci.CredentialsProvider, verbose bool, transport http.RoundTripper) *tekton.PipelinesProvider {
+func newTektonPipelinesProvider(kc *k8s.Client, creds oci.CredentialsProvider, verbose bool, transport http.RoundTripper) *tekton.PipelinesProvider {
 	options := []tekton.Opt{
 		tekton.WithCredentialsProvider(creds),
 		tekton.WithVerbose(verbose),
-		tekton.WithPipelineDecorator(deployDecorator{}),
+		tekton.WithPipelineDecorator(deployDecorator{kc}),
 		tekton.WithTransport(transport),
 	}
 
-	return tekton.NewPipelinesProvider(options...)
+	return tekton.NewPipelinesProvider(kc, options...)
 }
 
-func newKnativeDeployer(verbose bool) fn.Deployer {
-	options := []knative.DeployerOpt{
+func newKnativeDeployer(kc *k8s.Client, verbose bool) fn.Deployer {
+	return knative.NewDeployer(kc,
 		knative.WithDeployerVerbose(verbose),
-		knative.WithDeployerDecorator(deployDecorator{}),
-	}
-
-	return knative.NewDeployer(options...)
+		knative.WithDeployerDecorator(deployDecorator{kc}),
+	)
 }
 
 // newK8sDeployer builds the raw deployer.
 //
 // The Exposer is attached unconditionally, not only when the deploy asks for a
 // Route. The record saying whether teardown is owed lives on the cluster, so
 // wiring time cannot know.
-func newK8sDeployer(verbose bool) fn.Deployer {
-	return k8s.NewDeployer(
+func newK8sDeployer(kc *k8s.Client, verbose bool) fn.Deployer {
+	return k8s.NewDeployer(kc,
 		k8s.WithDeployerVerbose(verbose),
-		k8s.WithDeployerDecorator(deployDecorator{}),
+		k8s.WithDeployerDecorator(deployDecorator{kc}),
 		k8s.WithExposer(ocproute.New(deployers.Kubernetes)),
 	)
 }
@@ -194,28 +206,30 @@ func newK8sDeployer(verbose bool) fn.Deployer {
 // bypassing it. Attached unconditionally for the reason in newK8sDeployer,
 // which bites harder here: keda's Route has no owner reference, so a Route
 // nothing goes looking for is a Route nothing ever removes.
-func newKedaDeployer(verbose bool) fn.Deployer {
-	return keda.NewDeployer(
+func newKedaDeployer(kc *k8s.Client, verbose bool) fn.Deployer {
+	return keda.NewDeployer(kc,
 		keda.WithDeployerVerbose(verbose),
-		keda.WithDeployerDecorator(deployDecorator{}),
+		keda.WithDeployerDecorator(deployDecorator{kc}),
 		keda.WithExposer(ocproute.New(deployers.Keda)),
 	)
 }
 
+// deployDecorator adds OpenShift metadata when the target cluster is
+// OpenShift.
 type deployDecorator struct {
-	oshDec k8s.OpenshiftMetadataDecorator
+	kc *k8s.Client
 }
 
 func (d deployDecorator) UpdateAnnotations(function fn.Function, annotations map[string]string) map[string]string {
-	if k8s.IsOpenShift() {
-		return d.oshDec.UpdateAnnotations(function, annotations)
+	if ok, _ := d.kc.IsOpenShift(); ok {
+		return k8s.OpenshiftMetadataDecorator{}.UpdateAnnotations(function, annotations)
 	}
 	return annotations
 }
 
 func (d deployDecorator) UpdateLabels(function fn.Function, labels map[string]string) map[string]string {
-	if k8s.IsOpenShift() {
-		return d.oshDec.UpdateLabels(function, labels)
+	if ok, _ := d.kc.IsOpenShift(); ok {
+		return k8s.OpenshiftMetadataDecorator{}.UpdateLabels(function, labels)
 	}
 	return labels
 }

cmd/completion_util.go

@@ -16,7 +16,7 @@ import (
 )
 
 func CompleteFunctionList(cmd *cobra.Command, args []string, toComplete string) (strings []string, directive cobra.ShellCompDirective) {
-	kc := k8s.NewClient(k8s.GetClientConfig())
+	kc := k8s.NewClientFromKubeconfig()
 	listers := []fn.Lister{
 		knative.NewLister(kc, false),
 		k8s.NewLister(kc, false),

cmd/config.go

@@ -12,6 +12,7 @@ import (
 	"knative.dev/func/pkg/ci/github"
 	"knative.dev/func/pkg/config"
 	fn "knative.dev/func/pkg/functions"
+	"knative.dev/func/pkg/k8s"
 )
 
 func NewConfigCmd(
@@ -102,9 +103,9 @@ func runConfigCmd(cmd *cobra.Command, args []string) (err error) {
 	case "Add":
 		switch answers.SelectedConfig {
 		case "Volumes":
-			err = runAddVolumesPrompt(cmd.Context(), function)
+			err = runAddVolumesPrompt(cmd.Context(), k8s.NewClientFromKubeconfig(), function)
 		case "Environment variables":
-			err = runAddEnvsPrompt(cmd.Context(), function)
+			err = runAddEnvsPrompt(cmd.Context(), k8s.NewClientFromKubeconfig(), function)
 		case "Labels":
 			err = runAddLabelsPrompt(cmd.Context(), function, common.DefaultLoaderSaver)
 		case "Git":

cmd/config_envs.go

@@ -130,7 +130,7 @@ set environment variable from a secret
 				return loadSaver.Save(function)
 			}
 
-			return runAddEnvsPrompt(cmd.Context(), function)
+			return runAddEnvsPrompt(cmd.Context(), k8s.NewClientFromKubeconfig(), function)
 		},
 	}
 
@@ -211,7 +211,7 @@ func listEnvs(f fn.Function, w io.Writer, outputFormat Format) error {
 	}
 }
 
-func runAddEnvsPrompt(ctx context.Context, f fn.Function) (err error) {
+func runAddEnvsPrompt(ctx context.Context, kc *k8s.Client, f fn.Function) (err error) {
 
 	insertToIndex := 0
 
@@ -243,11 +243,11 @@ func runAddEnvsPrompt(ctx context.Context, f fn.Function) (err error) {
 	}
 
 	// SECTION - select the type of Environment variable to be added
-	secrets, err := k8s.ListSecretsNamesIfConnected(ctx, f.Deploy.Namespace)
+	secrets, err := k8s.ListSecretsNamesIfConnected(ctx, kc, f.Deploy.Namespace)
 	if err != nil {
 		return
 	}
-	configMaps, err := k8s.ListConfigMapsNamesIfConnected(ctx, f.Deploy.Namespace)
+	configMaps, err := k8s.ListConfigMapsNamesIfConnected(ctx, kc, f.Deploy.Namespace)
 	if err != nil {
 		return
 	}

cmd/config_volumes.go

@@ -98,7 +98,7 @@ For non-interactive usage, use flags to specify the volume type and configuratio
 			}
 
 			// Fall back to interactive mode
-			return runAddVolumesPrompt(cmd.Context(), function)
+			return runAddVolumesPrompt(cmd.Context(), k8s.NewClientFromKubeconfig(), function)
 		},
 	}
 
@@ -164,17 +164,17 @@ func listVolumes(f fn.Function) {
 	}
 }
 
-func runAddVolumesPrompt(ctx context.Context, f fn.Function) (err error) {
+func runAddVolumesPrompt(ctx context.Context, kc *k8s.Client, f fn.Function) (err error) {
 
-	secrets, err := k8s.ListSecretsNamesIfConnected(ctx, f.Deploy.Namespace)
+	secrets, err := k8s.ListSecretsNamesIfConnected(ctx, kc, f.Deploy.Namespace)
 	if err != nil {
 		return
 	}
-	configMaps, err := k8s.ListConfigMapsNamesIfConnected(ctx, f.Deploy.Namespace)
+	configMaps, err := k8s.ListConfigMapsNamesIfConnected(ctx, kc, f.Deploy.Namespace)
 	if err != nil {
 		return
 	}
-	persistentVolumeClaims, err := k8s.ListPersistentVolumeClaimsNamesIfConnected(ctx, f.Deploy.Namespace)
+	persistentVolumeClaims, err := k8s.ListPersistentVolumeClaimsNamesIfConnected(ctx, kc, f.Deploy.Namespace)
 	if err != nil {
 		return
 	}