From 798f46e891a33dd8e7d67c03b5b1a8bdd410e60e Mon Sep 17 00:00:00 2001 From: gauron99 Date: Wed, 26 Aug 2026 21:54:05 +0200 Subject: [PATCH 1/3] Introduce k8s.Client and inject it from the CLI Add k8s.Client, a single cluster client built once at the top of the call chain and passed down. It wraps clientcmd.ClientConfig and owns OpenShift detection, cached per instance, with WithOpenShift for tests. The CLI resolves the client in the command (build, deploy, run) or in cmd.NewClient via the new ClientConfig.K8sClient field, and hands it to the transport, the credentials provider, and the deploy decorator. The OpenShift helpers GetOpenShiftServiceCA and GetOpenShiftDockerCredentialLoaders become methods and their global forms are removed. Package-level functions still used by code not yet migrated (pkg/k8s internals, pkg/config, knative deployer) stay as they are and are marked for removal; no new callers were added. Tests use a fake API server (testing.FakeCluster) instead of the package-level SetOpenShiftForTest override. --- cmd/build.go | 16 +- cmd/client.go | 88 +++++++---- cmd/deploy.go | 28 ++-- cmd/deploy_test.go | 20 +-- cmd/func-util/main.go | 25 +-- cmd/run.go | 6 +- e2e/e2e_expose_test.go | 4 +- .../testing/integration_test_helper.go | 4 +- pkg/functions/client_int_test.go | 15 +- pkg/http/openshift.go | 4 +- pkg/http/openshift_int_test.go | 5 +- pkg/http/transport.go | 8 +- pkg/k8s/client.go | 142 ++++++++++++++++-- pkg/k8s/client_test.go | 108 +++++++++++-- pkg/k8s/deployer.go | 30 ++-- pkg/k8s/deployer_int_test.go | 55 ++++--- pkg/k8s/deployer_test.go | 20 +-- pkg/k8s/describer.go | 10 +- pkg/k8s/describer_int_test.go | 7 +- pkg/k8s/lister_int_test.go | 6 +- pkg/k8s/openshift.go | 36 +++-- pkg/k8s/remover.go | 9 +- pkg/k8s/remover_int_test.go | 6 +- pkg/keda/client.go | 7 +- pkg/keda/deployer.go | 16 +- pkg/keda/deployer_int_test.go | 55 ++++--- pkg/keda/deployer_unit_test.go | 10 +- pkg/keda/describer.go | 12 +- pkg/keda/describer_int_test.go | 8 +- pkg/keda/lister.go | 2 +- pkg/keda/lister_int_test.go | 6 +- pkg/keda/remover.go | 9 +- pkg/keda/remover_int_test.go | 6 +- pkg/knative/client.go | 14 +- pkg/knative/deployer.go | 30 ++-- pkg/knative/deployer_int_test.go | 55 ++++--- pkg/knative/describer.go | 14 +- pkg/knative/describer_int_test.go | 8 +- pkg/knative/lister.go | 2 +- pkg/knative/lister_int_test.go | 6 +- pkg/knative/remover.go | 10 +- pkg/knative/remover_int_test.go | 6 +- pkg/pipelines/tekton/pipelines_int_test.go | 12 +- pkg/pipelines/tekton/pipelines_provider.go | 13 +- pkg/testing/testing.go | 63 ++++++++ 45 files changed, 711 insertions(+), 305 deletions(-) diff --git a/cmd/build.go b/cmd/build.go index f7f4e25822..78cf6372b0 100644 --- a/cmd/build.go +++ b/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,14 +443,14 @@ 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: @@ -456,7 +458,7 @@ func (c buildConfig) clientOptions() ([]fn.Option, error) { 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))), ) diff --git a/cmd/client.go b/cmd/client.go index 015ab1c3cb..9b6ab5cfad 100644 --- a/cmd/client.go +++ b/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), @@ -107,10 +112,26 @@ 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 { + opts := []fnhttp.Option{ + fnhttp.WithInsecureSkipVerify(insecureSkipVerify), + fnhttp.WithOpenShiftServiceCA(kc), + } + if kc != nil && kc.Loader() != nil { + opts = append(opts, fnhttp.WithInClusterDialer(k8s.NewLazyInitInClusterDialer(kc.Loader()))) + } + return fnhttp.NewRoundTripper(opts...) } // newCredentialsProvider returns a credentials provider which possibly @@ -118,8 +139,8 @@ func newTransport(insecureSkipVerify bool) fnhttp.RoundTripCloser { // 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,24 +177,23 @@ 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), + tekton.WithK8sClient(kc), } return tekton.NewPipelinesProvider(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. @@ -181,10 +201,10 @@ func newKnativeDeployer(verbose bool) fn.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 +214,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 } diff --git a/cmd/deploy.go b/cmd/deploy.go index 37020c9e20..a6ad16a662 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -317,11 +317,15 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { return } + // The cluster client every cluster-facing component uses for this + // command. Resolved once, here, and passed down. + kc := k8s.NewClientFromKubeconfig() + // A Route is an OpenShift-only resource, not compatible with knative deployer. // Dont error here, knative + expose=route means expose key is ignored and // we print warning in warnExposeIgnore() if f.Expose == fn.ExposeRoute && f.Deployer != deployers.Knative { - ok, probeErr := k8s.DetectOpenShift() + ok, probeErr := kc.IsOpenShift() if probeErr != nil { return fmt.Errorf("--expose=route requires an OpenShift cluster, and this one "+ "could not be reached to check: %w. Fix the connection, or use --expose=none "+ @@ -345,7 +349,7 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { // also update the registry because there is a registry per namespace, // and their name includes the namespace. // This saves needing a manual flag ``--registry={destination namespace registry}`` - if changingNamespace(f) && k8s.IsOpenShift() && k8s.IsOpenShiftInternalRegistry(f.Registry) { + if ok, _ := kc.IsOpenShift(); changingNamespace(f) && ok && k8s.IsOpenShiftInternalRegistry(f.Registry) { f.Registry = "image-registry.openshift-image-registry.svc:5000/" + f.Namespace if cfg.Verbose { fmt.Fprintf(cmd.OutOrStdout(), "Info: Overriding openshift registry to %s\n", f.Registry) @@ -356,11 +360,11 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { printDeployMessages(cmd.OutOrStdout(), f) // create client with options from cfg - clientOptions, err := cfg.clientOptions() + clientOptions, err := cfg.clientOptions(kc) if err != nil { return } - client, done := newClient(ClientConfig{Verbose: cfg.Verbose, InsecureSkipVerify: cfg.RegistryInsecure}, clientOptions...) + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, InsecureSkipVerify: cfg.RegistryInsecure, K8sClient: kc}, clientOptions...) defer done() // Deploy @@ -856,28 +860,28 @@ func (c deployConfig) Validate(cmd *cobra.Command) (err error) { // clientOptions returns client options specific to deploy, including the // appropriate deployer -func (c deployConfig) clientOptions() ([]fn.Option, error) { +func (c deployConfig) clientOptions(kc *k8s.Client) ([]fn.Option, error) { // Start with build config options - o, err := c.buildConfig.clientOptions() + o, err := c.buildConfig.clientOptions(kc) if err != nil { return o, err } - 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) // Override the pipelines provider to use custom credentials // This is needed for remote builds (deploy --remote) - o = append(o, fn.WithPipelinesProvider(newTektonPipelinesProvider(creds, c.Verbose, t))) + o = append(o, fn.WithPipelinesProvider(newTektonPipelinesProvider(kc, creds, c.Verbose, t))) // Add the appropriate deployer based on deploy type. switch c.Deployer { case knative.KnativeDeployerName: - o = append(o, fn.WithDeployer(newKnativeDeployer(c.Verbose))) + o = append(o, fn.WithDeployer(newKnativeDeployer(kc, c.Verbose))) case k8s.KubernetesDeployerName: - o = append(o, fn.WithDeployer(newK8sDeployer(c.Verbose))) + o = append(o, fn.WithDeployer(newK8sDeployer(kc, c.Verbose))) case keda.KedaDeployerName: - o = append(o, fn.WithDeployer(newKedaDeployer(c.Verbose))) + o = append(o, fn.WithDeployer(newKedaDeployer(kc, c.Verbose))) default: return o, fmt.Errorf("unsupported deploy type: %s (supported: %s, %s, %s)", c.Deployer, knative.KnativeDeployerName, k8s.KubernetesDeployerName, keda.KedaDeployerName) } diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index a32ac823be..0d7e9b45cb 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -1355,8 +1355,7 @@ func TestDeploy_BasicRedeployPipelinesCorrectNamespace(t *testing.T) { func TestDeploy_NamespaceChangePreservesExternalRegistry(t *testing.T) { root := FromTempDirectory(t) - cleanup := k8s.SetOpenShiftForTest(true, nil) - defer cleanup() + FakeCluster(t, true) // Create a function deployed to "ns1" with an external registry f := fn.Function{ @@ -1390,8 +1389,7 @@ func TestDeploy_NamespaceChangePreservesExternalRegistry(t *testing.T) { func TestDeploy_NamespaceChangeUpdatesInternalRegistry(t *testing.T) { root := FromTempDirectory(t) - cleanup := k8s.SetOpenShiftForTest(true, nil) - defer cleanup() + FakeCluster(t, true) // Create a function deployed to "ns1" using the internal registry f := fn.Function{ @@ -2860,9 +2858,8 @@ func TestDeploy_ExposeInvalidValueError(t *testing.T) { // (Function.Expose) and observed status (Deploy.Expose) end-to-end. func TestDeploy_ExposeRoutePersists(t *testing.T) { root := FromTempDirectory(t) - // CLI gates route on OpenShift; tests run without a cluster. - cleanup := k8s.SetOpenShiftForTest(true, nil) - defer cleanup() + // CLI gates route on OpenShift; a fake cluster answers the probe. + FakeCluster(t, true) if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}); err != nil { t.Fatal(err) @@ -2930,8 +2927,7 @@ func TestDeploy_ExposeIgnoredByDeployerNote(t *testing.T) { t.Run(tt.name, func(t *testing.T) { root := FromTempDirectory(t) // route cases need OpenShift gate open; none/empty do not care. - cleanup := k8s.SetOpenShiftForTest(true, nil) - defer cleanup() + FakeCluster(t, true) if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}); err != nil { t.Fatal(err) } @@ -2988,8 +2984,7 @@ func TestDeploy_RemoteExposeRecordsObservation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { root := FromTempDirectory(t) - cleanup := k8s.SetOpenShiftForTest(true, nil) - defer cleanup() + FakeCluster(t, true) if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}); err != nil { t.Fatal(err) @@ -3045,8 +3040,7 @@ func TestDeploy_RemoteExposeRecordsObservation(t *testing.T) { func TestDeploy_ExposeRouteUnreachableClusterIsNotAPlatformClaim(t *testing.T) { root := FromTempDirectory(t) - cleanup := k8s.SetOpenShiftForTest(false, errors.New("dial tcp 127.0.0.1:6443: connect: connection refused")) - defer cleanup() + UnreachableCluster(t) if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}); err != nil { t.Fatal(err) diff --git a/cmd/func-util/main.go b/cmd/func-util/main.go index 41c01e4124..ec4916df3f 100644 --- a/cmd/func-util/main.go +++ b/cmd/func-util/main.go @@ -164,22 +164,23 @@ func deploy(ctx context.Context) error { if deployer == "" { deployer = knative.KnativeDeployerName } + kc := k8s.NewClientFromKubeconfig() var d fn.Deployer switch deployer { case knative.KnativeDeployerName: - d = knative.NewDeployer( - knative.WithDeployerDecorator(deployDecorator{}), + d = knative.NewDeployer(kc, + knative.WithDeployerDecorator(deployDecorator{kc}), knative.WithDeployerVerbose(true), ) case k8s.KubernetesDeployerName: - d = k8s.NewDeployer( - k8s.WithDeployerDecorator(deployDecorator{}), + d = k8s.NewDeployer(kc, + k8s.WithDeployerDecorator(deployDecorator{kc}), k8s.WithDeployerVerbose(true), k8s.WithExposer(ocproute.New(deployers.Kubernetes)), ) case keda.KedaDeployerName: - d = keda.NewDeployer( - keda.WithDeployerDecorator(deployDecorator{}), + d = keda.NewDeployer(kc, + keda.WithDeployerDecorator(deployDecorator{kc}), keda.WithDeployerVerbose(true), keda.WithExposer(ocproute.New(deployers.Keda)), ) @@ -192,7 +193,7 @@ func deploy(ctx context.Context) error { // on-cluster in func.yaml. Same rule and same split as the CLI gate; only // the wording differs (the user is holding func.yaml here, not a flag). if f.Expose == fn.ExposeRoute && deployer != knative.KnativeDeployerName { - ok, probeErr := k8s.DetectOpenShift() + ok, probeErr := kc.IsOpenShift() if probeErr != nil { return fmt.Errorf("expose is %q but this cluster could not be asked whether it "+ "serves route.openshift.io: %w", f.Expose, probeErr) @@ -214,19 +215,19 @@ func deploy(ctx context.Context) error { } 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 } diff --git a/cmd/run.go b/cmd/run.go index f60ef3e9aa..f8a4cafff9 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -16,6 +16,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" ) @@ -190,7 +191,8 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { } // Client - clientOptions, err := cfg.clientOptions() + kc := k8s.NewClientFromKubeconfig() + clientOptions, err := cfg.clientOptions(kc) if err != nil { return } @@ -201,7 +203,7 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { clientOptions = append(clientOptions, fn.WithStartTimeout(cfg.StartTimeout)) } - client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, clientOptions...) + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, K8sClient: kc}, clientOptions...) defer done() // Build diff --git a/e2e/e2e_expose_test.go b/e2e/e2e_expose_test.go index aad8c367a7..85a84ec286 100644 --- a/e2e/e2e_expose_test.go +++ b/e2e/e2e_expose_test.go @@ -611,7 +611,7 @@ func TestExpose_KedaRouteDomain(t *testing.T) { t.Errorf("expected spec.host %q, got %q", domain, host) } - hsoClient, err := keda.NewHTTPScaledObjectClientset() + hsoClient, err := keda.NewHTTPScaledObjectClientset(k8s.NewClientFromKubeconfig()) if err != nil { t.Fatal(err) } @@ -884,7 +884,7 @@ func TestExpose_RemoteKedaRoute(t *testing.T) { t.Fatalf("expected 1 Route in interceptor namespace %q, found %d", interceptorNS, n) } - hsoClient, err := keda.NewHTTPScaledObjectClientset() + hsoClient, err := keda.NewHTTPScaledObjectClientset(k8s.NewClientFromKubeconfig()) if err != nil { t.Fatal(err) } diff --git a/pkg/deployer/testing/integration_test_helper.go b/pkg/deployer/testing/integration_test_helper.go index baaad2ea77..42f7478879 100644 --- a/pkg/deployer/testing/integration_test_helper.go +++ b/pkg/deployer/testing/integration_test_helper.go @@ -1064,7 +1064,7 @@ func createTrigger(t *testing.T, ctx context.Context, namespace, triggerName str }, }, } - eventingClient, err := knative.NewEventingClient(namespace) + eventingClient, err := knative.NewEventingClient(k8s.NewClient(k8s.GetClientConfig()), namespace) if err != nil { t.Fatal(err) } @@ -1167,7 +1167,7 @@ func deferCleanup(t *testing.T, namespace string, resourceType string, name stri }) case "trigger": t.Cleanup(func() { - if eventingClient, err := knative.NewEventingClient(namespace); err == nil { + if eventingClient, err := knative.NewEventingClient(k8s.NewClient(k8s.GetClientConfig()), namespace); err == nil { _ = eventingClient.DeleteTrigger(context.Background(), name) } }) diff --git a/pkg/functions/client_int_test.go b/pkg/functions/client_int_test.go index 9f47dee8d5..e028665cdd 100644 --- a/pkg/functions/client_int_test.go +++ b/pkg/functions/client_int_test.go @@ -206,13 +206,14 @@ func TestInt_Deploy_WithTriggers(t *testing.T) { } func TestInt_Update_WithAnnotationsAndLabels(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) resetEnv() _, cleanup := Mktemp(t) defer cleanup() functionName := "updateannlab" verbose := false - servingClient, err := knative.NewServingClient(DefaultIntTestNamespace) + servingClient, err := knative.NewServingClient(kc, DefaultIntTestNamespace) if err != nil { t.Fatal(err) } @@ -673,9 +674,9 @@ func newClient(verbose bool) *fn.Client { fn.WithScaffolder(oci.NewScaffolder(true)), fn.WithBuilder(oci.NewBuilder("", verbose)), fn.WithPusher(oci.NewPusher(true, true, verbose)), - fn.WithDeployer(knative.NewDeployer(knative.WithDeployerVerbose(verbose))), - fn.WithDescribers(knative.NewDescriber(verbose), k8s.NewDescriber(verbose)), - fn.WithRemovers(knative.NewRemover(verbose), k8s.NewRemover(verbose)), + fn.WithDeployer(knative.NewDeployer(kc, knative.WithDeployerVerbose(verbose))), + fn.WithDescribers(knative.NewDescriber(kc, verbose), k8s.NewDescriber(kc, verbose)), + fn.WithRemovers(knative.NewRemover(kc, verbose), k8s.NewRemover(kc, verbose)), fn.WithListers(knative.NewLister(kc, verbose), k8s.NewLister(kc, verbose)), fn.WithVerbose(verbose), ) @@ -691,9 +692,9 @@ func newClientWithS2i(verbose bool) *fn.Client { fn.WithScaffolder(s2i.NewScaffolder(true)), fn.WithBuilder(s2i.NewBuilder(s2i.WithVerbose(verbose))), fn.WithPusher(docker.NewPusher(docker.WithVerbose(verbose), docker.WithInsecure(true))), - fn.WithDeployer(knative.NewDeployer(knative.WithDeployerVerbose(verbose))), - fn.WithDescribers(knative.NewDescriber(verbose), k8s.NewDescriber(verbose)), - fn.WithRemovers(knative.NewRemover(verbose), k8s.NewRemover(verbose)), + fn.WithDeployer(knative.NewDeployer(kc, knative.WithDeployerVerbose(verbose))), + fn.WithDescribers(knative.NewDescriber(kc, verbose), k8s.NewDescriber(kc, verbose)), + fn.WithRemovers(knative.NewRemover(kc, verbose), k8s.NewRemover(kc, verbose)), fn.WithListers(knative.NewLister(kc, verbose), k8s.NewLister(kc, verbose)), ) } diff --git a/pkg/http/openshift.go b/pkg/http/openshift.go index 0d9b5d8939..7c6be7104b 100644 --- a/pkg/http/openshift.go +++ b/pkg/http/openshift.go @@ -13,7 +13,7 @@ import ( const openShiftRegistryHost = "image-registry.openshift-image-registry.svc" // WithOpenShiftServiceCA enables trust to OpenShift's service CA for internal image registry -func WithOpenShiftServiceCA() Option { +func WithOpenShiftServiceCA(c *k8s.Client) Option { var err error var ca *x509.Certificate var o sync.Once @@ -21,7 +21,7 @@ func WithOpenShiftServiceCA() Option { selectCA := func(ctx context.Context, serverName string) (*x509.Certificate, error) { if strings.HasPrefix(serverName, openShiftRegistryHost) { o.Do(func() { - ca, err = k8s.GetOpenShiftServiceCA(ctx) + ca, err = c.OpenShiftServiceCA(ctx) if err != nil { err = fmt.Errorf("cannot get CA: %w", err) } diff --git a/pkg/http/openshift_int_test.go b/pkg/http/openshift_int_test.go index 2cc6c5e30c..b26f19ada7 100644 --- a/pkg/http/openshift_int_test.go +++ b/pkg/http/openshift_int_test.go @@ -11,12 +11,13 @@ import ( ) func TestInt_RoundTripper(t *testing.T) { - if !k8s.IsOpenShift() { + kc := k8s.NewClientFromKubeconfig() + if ok, _ := kc.IsOpenShift(); !ok { t.Skip("The cluster in not an instance of OpenShift.") return } - transport := fnhttp.NewRoundTripper(fnhttp.WithOpenShiftServiceCA()) + transport := fnhttp.NewRoundTripper(fnhttp.WithOpenShiftServiceCA(kc)) defer transport.Close() client := http.Client{ diff --git a/pkg/http/transport.go b/pkg/http/transport.go index 6424d766e0..f651f1d1ce 100644 --- a/pkg/http/transport.go +++ b/pkg/http/transport.go @@ -50,10 +50,12 @@ func WithInsecureSkipVerify(insecureSkipVerify bool) Option { } } -// NewRoundTripper returns new closable RoundTripper that first tries to dial connection in standard way, -// if the dial operation fails due to hostname resolution the RoundTripper tries to dial from in cluster pod. +// NewRoundTripper returns new closable RoundTripper that first tries to dial +// connection in standard way, if the dial operation fails due to hostname +// resolution the RoundTripper tries to dial from in cluster pod. // -// This is useful for accessing cluster internal services (pushing a CloudEvent into Knative broker). +// This is useful for accessing cluster internal services (pushing a CloudEvent +// into Knative broker). func NewRoundTripper(opts ...Option) RoundTripCloser { o := options{ inClusterDialer: k8s.NewLazyInitInClusterDialer(k8s.GetClientConfig()), diff --git a/pkg/k8s/client.go b/pkg/k8s/client.go index be45474cb4..f8b15bf785 100644 --- a/pkg/k8s/client.go +++ b/pkg/k8s/client.go @@ -5,11 +5,13 @@ import ( "sync" "time" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" _ "k8s.io/client-go/plugin/pkg/client/auth" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) const ( @@ -17,23 +19,68 @@ const ( DefaultErrorWindowTimeout = 2 * time.Second ) +// Client is the single entry point for cluster access. It is constructed +// once, at the top of the call chain (the CLI), and passed down to every +// component which talks to the cluster. Components must NOT construct their +// own cluster configuration. type Client struct { - cc clientcmd.ClientConfig - cfg *rest.Config - cfgErr error - o sync.Once + cc clientcmd.ClientConfig + + cfg *rest.Config + cfgErr error + cfgOnce sync.Once + + // isOpenShift answers IsOpenShift: memoized detection, or a preset. + isOpenShift func() (bool, error) } -func NewClient(cc clientcmd.ClientConfig) *Client { - return &Client{cc: cc} +// ClientOpt configures a Client. +type ClientOpt func(*Client) + +// WithOpenShift sets the OpenShift detection result up front, so the Client +// never contacts the cluster to find out. Intended for tests. +func WithOpenShift(v bool) ClientOpt { + return func(c *Client) { + c.isOpenShift = func() (bool, error) { return v, nil } + } } -func NewClientFromConfig(cfg *rest.Config) *Client { - return &Client{cfg: cfg} +func applyClientOpts(c *Client, opts []ClientOpt) *Client { + if c.isOpenShift == nil { + c.isOpenShift = sync.OnceValues(func() (bool, error) { return detectOpenShift(c) }) + } + for _, o := range opts { + o(c) + } + return c } -func (c *Client) ClientConfig() (*rest.Config, error) { - c.o.Do(func() { +// NewClient returns a Client backed by the given client configuration. +func NewClient(cc clientcmd.ClientConfig, opts ...ClientOpt) *Client { + return applyClientOpts(&Client{cc: cc}, opts) +} + +// NewClientFromKubeconfig returns a Client which resolves its configuration +// the way kubectl does: KUBECONFIG, ~/.kube/config, in-cluster. +func NewClientFromKubeconfig(opts ...ClientOpt) *Client { + return NewClient(GetClientConfig(), opts...) +} + +// NewClientFromConfig returns a Client backed by an already-resolved rest +// config. Used in tests that do not want to go through kubeconfig. +func NewClientFromConfig(cfg *rest.Config, opts ...ClientOpt) *Client { + return applyClientOpts(&Client{cfg: cfg}, opts) +} + +// Loader returns the deferred kubeconfig loader, if this Client was built +// from kubeconfig. Nil when built from NewClientFromConfig. +func (c *Client) Loader() clientcmd.ClientConfig { + return c.cc +} + +// RestConfig returns the resolved rest configuration (host, token, certs). +func (c *Client) RestConfig() (*rest.Config, error) { + c.cfgOnce.Do(func() { if c.cfg != nil { return } @@ -49,14 +96,85 @@ func (c *Client) ClientConfig() (*rest.Config, error) { return c.cfg, c.cfgErr } +// RawConfig returns the merged kubeconfig. +func (c *Client) RawConfig() (clientcmdapi.Config, error) { + if c.cc == nil { + return clientcmdapi.Config{}, fmt.Errorf("no kubernetes client configuration available") + } + return c.cc.RawConfig() +} + +// Clientset returns a typed clientset. func (c *Client) Clientset() (*kubernetes.Clientset, error) { - cfg, err := c.ClientConfig() + cfg, err := c.RestConfig() if err != nil { - return nil, err + return nil, fmt.Errorf("failed to create new kubernetes client: %w", err) } return kubernetes.NewForConfig(cfg) } +// DynamicClient returns a dynamic client. +func (c *Client) DynamicClient() (dynamic.Interface, error) { + cfg, err := c.RestConfig() + if err != nil { + return nil, fmt.Errorf("failed to create new kubernetes client: %w", err) + } + return dynamic.NewForConfig(cfg) +} + +// DefaultNamespace returns the namespace of the active context. +func (c *Client) DefaultNamespace() (string, error) { + if c.cc == nil { + return "", fmt.Errorf("no kubernetes client configuration available") + } + ns, _, err := c.cc.Namespace() + return ns, err +} + +// ClientAndNamespace returns a clientset and the namespace to use: ns if +// given, else the namespace of the active context. +func (c *Client) ClientAndNamespace(ns string) (*kubernetes.Clientset, string, error) { + var err error + if ns == "" { + if ns, err = c.DefaultNamespace(); err != nil { + return nil, ns, err + } + } + cs, err := c.Clientset() + return cs, ns, err +} + +// IsOpenShift reports whether the cluster serves the OpenShift Route API. +// A non-nil error means the cluster could not be asked and the bool is +// meaningless. Detection runs at most once per Client, on first use. +func (c *Client) IsOpenShift() (bool, error) { + return c.isOpenShift() +} + +// detectOpenShift asks the API server for the route.openshift.io/v1 group, +// which works even under restrictive RBAC. Any error, including an +// unreachable cluster, counts as "not OpenShift". +func detectOpenShift(c *Client) (bool, error) { + cs, err := c.Clientset() + if err != nil { + return false, err + } + _, err = cs.Discovery().ServerResourcesForGroupVersion("route.openshift.io/v1") + switch { + case err == nil: + return true, nil + case apierrors.IsNotFound(err): + // The cluster answered: it does not serve this API. + return false, nil + default: + return false, err + } +} + +// The functions below read the kubeconfig ad hoc. They remain only for +// callers not yet migrated to Client and will be removed once none are left. +// Do not add new callers. + func NewClientAndResolvedNamespace(ns string) (*kubernetes.Clientset, string, error) { var err error if ns == "" { diff --git a/pkg/k8s/client_test.go b/pkg/k8s/client_test.go index 5f96d61aea..d2cbeafb2b 100644 --- a/pkg/k8s/client_test.go +++ b/pkg/k8s/client_test.go @@ -1,4 +1,4 @@ -package k8s +package k8s_test import ( "fmt" @@ -7,24 +7,27 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + + "knative.dev/func/pkg/k8s" + . "knative.dev/func/pkg/testing" ) func TestNewClientFromConfig(t *testing.T) { cfg := &rest.Config{Host: "https://example.com:6443"} - c := NewClientFromConfig(cfg) + c := k8s.NewClientFromConfig(cfg) - got, err := c.ClientConfig() + got, err := c.RestConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } if got != cfg { - t.Error("ClientConfig() should return the same config that was passed in") + t.Error("RestConfig() should return the same config that was passed in") } } func TestClientClientset(t *testing.T) { cfg := &rest.Config{Host: "https://example.com:6443"} - c := NewClientFromConfig(cfg) + c := k8s.NewClientFromConfig(cfg) cs, err := c.Clientset() if err != nil { @@ -37,20 +40,20 @@ func TestClientClientset(t *testing.T) { func TestNewClientWithInvalidConfig(t *testing.T) { cc := &fakeClientConfig{err: fmt.Errorf("no kubeconfig")} - c := NewClient(cc) + c := k8s.NewClient(cc) - _, err := c.ClientConfig() + _, err := c.RestConfig() if err == nil { - t.Fatal("expected error when ClientConfig fails") + t.Fatal("expected error when RestConfig fails") } } func TestNewClientWithValidConfig(t *testing.T) { cfg := &rest.Config{Host: "https://example.com:6443"} cc := &fakeClientConfig{cfg: cfg} - c := NewClient(cc) + c := k8s.NewClient(cc) - got, err := c.ClientConfig() + got, err := c.RestConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -79,3 +82,88 @@ func (f *fakeClientConfig) Namespace() (string, bool, error) { func (f *fakeClientConfig) ConfigAccess() clientcmd.ConfigAccess { return nil } + +// TestClient_IsOpenShift verifies detection against a fake API server: true +// when the route.openshift.io/v1 group exists, false otherwise. +func TestClient_IsOpenShift(t *testing.T) { + for _, openshift := range []bool{true, false} { + FakeCluster(t, openshift) + if got, err := k8s.NewClientFromKubeconfig().IsOpenShift(); got != openshift || err != nil { + t.Errorf("fake cluster openshift=%v, IsOpenShift() = %v, %v", openshift, got, err) + } + } +} + +// TestClient_IsOpenShiftUnreachable verifies an unreachable cluster reports +// an error rather than claiming "not OpenShift". +func TestClient_IsOpenShiftUnreachable(t *testing.T) { + UnreachableCluster(t) + if ok, err := k8s.NewClientFromKubeconfig().IsOpenShift(); err == nil || ok { + t.Errorf("expected an error for an unreachable cluster, got %v, %v", ok, err) + } +} + +// TestClient_WithOpenShift verifies the preset answer wins and no detection +// runs, even though the fake cluster would answer the opposite. +func TestClient_WithOpenShift(t *testing.T) { + FakeCluster(t, false) + if ok, _ := k8s.NewClientFromKubeconfig(k8s.WithOpenShift(true)).IsOpenShift(); !ok { + t.Error("expected preset true") + } + FakeCluster(t, true) + if ok, _ := k8s.NewClientFromKubeconfig(k8s.WithOpenShift(false)).IsOpenShift(); ok { + t.Error("expected preset false") + } +} + +// TestClient_Namespace verifies namespace resolution from the active context. +func TestClient_Namespace(t *testing.T) { + FakeCluster(t, false) + c := k8s.NewClientFromKubeconfig() + + ns, err := c.DefaultNamespace() + if err != nil { + t.Fatal(err) + } + if ns != "default" { + t.Errorf("expected namespace 'default', got %q", ns) + } + + if _, ns, err = c.ClientAndNamespace(""); err != nil || ns != "default" { + t.Errorf("expected 'default', got %q (err %v)", ns, err) + } + if _, ns, err = c.ClientAndNamespace("other"); err != nil || ns != "other" { + t.Errorf("expected 'other', got %q (err %v)", ns, err) + } +} + +// TestClient_DefaultOpenShiftRegistry verifies the registry path uses the +// active namespace. +func TestClient_DefaultOpenShiftRegistry(t *testing.T) { + FakeCluster(t, true) + got := k8s.NewClientFromKubeconfig().DefaultOpenShiftRegistry() + want := "image-registry.openshift-image-registry.svc:5000/default" + if got != want { + t.Errorf("expected %q, got %q", want, got) + } +} + +// TestClient_OpenShiftDockerCredentialLoaders verifies the loader serves the +// active user's token for the internal registry only. +func TestClient_OpenShiftDockerCredentialLoaders(t *testing.T) { + FakeCluster(t, true) + loaders := k8s.NewClientFromKubeconfig().OpenShiftDockerCredentialLoaders() + if len(loaders) != 1 { + t.Fatalf("expected one loader, got %d", len(loaders)) + } + creds, err := loaders[0]("image-registry.openshift-image-registry.svc:5000") + if err != nil { + t.Fatal(err) + } + if creds.Username != "openshift" || creds.Password != "fake-token" { + t.Errorf("unexpected credentials %+v", creds) + } + if _, err = loaders[0]("docker.io"); err == nil { + t.Error("expected error for a foreign registry") + } +} diff --git a/pkg/k8s/deployer.go b/pkg/k8s/deployer.go index 8df73b6972..203b8c255b 100644 --- a/pkg/k8s/deployer.go +++ b/pkg/k8s/deployer.go @@ -63,12 +63,13 @@ type DeployerOpt func(*Deployer) type Deployer struct { verbose bool decorator deployer.DeployDecorator + kc *Client exposer deployer.Exposer } -func NewDeployer(opts ...DeployerOpt) *Deployer { - d := &Deployer{} +func NewDeployer(kc *Client, opts ...DeployerOpt) *Deployer { + d := &Deployer{kc: kc} for _, opt := range opts { opt(d) } @@ -93,7 +94,7 @@ func WithDeployerDecorator(decorator deployer.DeployDecorator) DeployerOpt { } } -func onClusterFix(f fn.Function) fn.Function { +func (d *Deployer) onClusterFix(f fn.Function) fn.Function { // This only exists because of a bootstrapping problem with On-Cluster // builds: It appears that, when sending a function to be built on-cluster // the target namespace is not being transmitted in the pipeline @@ -102,7 +103,7 @@ func onClusterFix(f fn.Function) fn.Function { // earlier versions of this logic relied entirely on the current // kubernetes context. if f.Namespace == "" && f.Deploy.Namespace == "" { - f.Namespace, _ = GetDefaultNamespace() + f.Namespace, _ = d.kc.DefaultNamespace() } return f } @@ -117,7 +118,10 @@ func newEventingClient(config *rest.Config, namespace string) (clienteventingv1. } func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResult, error) { - f = onClusterFix(f) + if d.kc == nil { + return fn.DeploymentResult{}, fmt.Errorf("kubernetes client is not initialized") + } + f = d.onClusterFix(f) // Choosing f.Namespace vs f.Deploy.Namespace: // This is minimal logic currently required of all deployer impls. // If f.Namespace is defined, this is the (possibly new) target @@ -142,18 +146,12 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu f.Deploy.Image = f.Build.Image } - // Get the Kubernetes REST config - config, err := GetClientConfig().ClientConfig() - if err != nil { - return fn.DeploymentResult{}, err - } - - clientset, err := kubernetes.NewForConfig(config) + clientset, err := d.kc.Clientset() if err != nil { return fn.DeploymentResult{}, err } - dynClient, err := dynamic.NewForConfig(config) + dynClient, err := d.kc.DynamicClient() if err != nil { return fn.DeploymentResult{}, fmt.Errorf("failed to create dynamic client: %w", err) } @@ -286,7 +284,11 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu } // Sync triggers - eventingClient, err := newEventingClient(config, namespace) + restConfig, err := d.kc.RestConfig() + if err != nil { + return fn.DeploymentResult{}, err + } + eventingClient, err := newEventingClient(restConfig, namespace) if err != nil { return fn.DeploymentResult{}, fmt.Errorf("failed to create eventing client: %w", err) } diff --git a/pkg/k8s/deployer_int_test.go b/pkg/k8s/deployer_int_test.go index b2d466f927..cf79b4f128 100644 --- a/pkg/k8s/deployer_int_test.go +++ b/pkg/k8s/deployer_int_test.go @@ -12,67 +12,74 @@ import ( func TestInt_FullPath(t *testing.T) { kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_FullPath(t, - k8s.NewDeployer(k8s.WithDeployerVerbose(false)), - k8s.NewRemover(false), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), + k8s.NewRemover(kc, false), k8s.NewLister(kc, false), - k8s.NewDescriber(false), + k8s.NewDescriber(kc, false), k8s.KubernetesDeployerName) } func TestInt_Deploy(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_Deploy(t, - k8s.NewDeployer(k8s.WithDeployerVerbose(false)), - k8s.NewRemover(false), - k8s.NewDescriber(false), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), + k8s.NewRemover(kc, false), + k8s.NewDescriber(kc, false), k8s.KubernetesDeployerName) } func TestInt_Metadata(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_Metadata(t, - k8s.NewDeployer(k8s.WithDeployerVerbose(false)), - k8s.NewRemover(false), - k8s.NewDescriber(false), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), + k8s.NewRemover(kc, false), + k8s.NewDescriber(kc, false), k8s.KubernetesDeployerName) } func TestInt_Events(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) t.Skip("Kubernetes deploy does not support func subscribe yet") deployertesting.TestInt_Events(t, - k8s.NewDeployer(k8s.WithDeployerVerbose(false)), - k8s.NewRemover(false), - k8s.NewDescriber(false), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), + k8s.NewRemover(kc, false), + k8s.NewDescriber(kc, false), k8s.KubernetesDeployerName) } func TestInt_Scale(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_Scale(t, - k8s.NewDeployer(k8s.WithDeployerVerbose(false)), - k8s.NewRemover(false), - k8s.NewDescriber(false), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), + k8s.NewRemover(kc, false), + k8s.NewDescriber(kc, false), k8s.KubernetesDeployerName) } func TestInt_EnvsUpdate(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_EnvsUpdate(t, - k8s.NewDeployer(k8s.WithDeployerVerbose(false)), - k8s.NewRemover(false), - k8s.NewDescriber(false), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), + k8s.NewRemover(kc, false), + k8s.NewDescriber(kc, false), k8s.KubernetesDeployerName) } func TestInt_ResourceValidationOnFirstDeploy(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_ResourceValidationOnFirstDeploy(t, - k8s.NewDeployer(k8s.WithDeployerVerbose(false)), - k8s.NewRemover(false), - k8s.NewDescriber(false), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), + k8s.NewRemover(kc, false), + k8s.NewDescriber(kc, false), k8s.KubernetesDeployerName) } func TestInt_OperatorSync(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_OperatorSync(t, - k8s.NewDeployer(k8s.WithDeployerVerbose(false)), - k8s.NewRemover(false), - k8s.NewDescriber(false), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), + k8s.NewRemover(kc, false), + k8s.NewDescriber(kc, false), k8s.KubernetesDeployerName) } diff --git a/pkg/k8s/deployer_test.go b/pkg/k8s/deployer_test.go index 8c6747fefd..780a36c4c6 100644 --- a/pkg/k8s/deployer_test.go +++ b/pkg/k8s/deployer_test.go @@ -594,7 +594,7 @@ func Test_ProcessVolumes_ValidPath(t *testing.T) { // state keda's embedded raw Deployer runs in on every deploy, so it must stay // quiet - keda exposes its own Route afterwards (pkg/keda/exposure.go). func Test_ResolveExposure_NoExposer(t *testing.T) { - d := NewDeployer() + d := NewDeployer(nil) // fake clientsets are passed explicitly below f := fn.Function{Name: "f", Deploy: fn.DeploySpec{Namespace: "ns"}} ctx := t.Context() clientset := fake.NewClientset() @@ -671,7 +671,7 @@ func Test_ResolveExposure_NoExposerLeavesHostnameAlone(t *testing.T) { clientset := fake.NewClientset(svc) dynClient := dynamicfakeclient.NewSimpleDynamicClient(runtime.NewScheme()) - d := NewDeployer() + d := NewDeployer(nil) // fake clientsets are passed explicitly below f := fn.Function{Name: "f", Expose: fn.ExposeNone, Deploy: fn.DeploySpec{Namespace: "ns"}} if _, applied, err := d.resolveExposure(ctx, f, "ns", svc, clientset, dynClient, nil, nil); err != nil { @@ -711,7 +711,7 @@ func Test_generateService_CarriesExposureRecordAcrossRedeploy(t *testing.T) { host = "f-ns.apps.example.com" routeNS = "openshift-keda" ) - d := NewDeployer() + d := NewDeployer(nil) // fake clientsets are passed explicitly below // A function already deployed to "ns". Each subtest regenerates its // Service as a redeploy would, against a different live Service. f := fn.Function{Name: "f", Deploy: fn.DeploySpec{Namespace: "ns"}} @@ -790,7 +790,7 @@ func Test_ResolveExposure_WithExposer(t *testing.T) { dynClient := dynamicfakeclient.NewSimpleDynamicClient(runtime.NewScheme()) exposer := &stubExposer{host: host} - d := NewDeployer(WithExposer(exposer)) + d := NewDeployer(nil, WithExposer(exposer)) f := fn.Function{Name: "f", Expose: fn.ExposeRoute, Deploy: fn.DeploySpec{Namespace: "ns"}} svc := testService(nil) @@ -842,7 +842,7 @@ func Test_ResolveExposure_WithExposer(t *testing.T) { dynClient := dynamicfakeclient.NewSimpleDynamicClient(runtime.NewScheme()) exposer := &stubExposer{} - d := NewDeployer(WithExposer(exposer)) + d := NewDeployer(nil, WithExposer(exposer)) f := fn.Function{Name: "f", Expose: fn.ExposeNone, Deploy: fn.DeploySpec{Namespace: "ns"}} _, applied, err := d.resolveExposure(ctx, f, "ns", svc, clientset, dynClient, nil, nil) @@ -890,7 +890,7 @@ func Test_ResolveExposure_RecordFailureRollsBack(t *testing.T) { t.Run("rollback happens and both facts are reported", func(t *testing.T) { exposer := &stubExposer{host: host} - d := NewDeployer(WithExposer(exposer)) + d := NewDeployer(nil, WithExposer(exposer)) _, _, err := d.resolveExposure(t.Context(), f, "ns", testService(nil), newClientset(), dynClient, nil, nil) if err == nil { @@ -906,7 +906,7 @@ func Test_ResolveExposure_RecordFailureRollsBack(t *testing.T) { t.Run("a failed rollback reports both failures", func(t *testing.T) { exposer := &stubExposer{host: host, unexposeErr: fmt.Errorf("also boom")} - d := NewDeployer(WithExposer(exposer)) + d := NewDeployer(nil, WithExposer(exposer)) _, _, err := d.resolveExposure(t.Context(), f, "ns", testService(nil), newClientset(), dynClient, nil, nil) if err == nil { @@ -964,7 +964,7 @@ func Test_ResolveExposure_RecordOrSilence(t *testing.T) { dynClient := dynamicfakeclient.NewSimpleDynamicClient(runtime.NewScheme()) exposer := &stubExposer{unexposeErr: tt.unexposeErr} - d := NewDeployer(WithExposer(exposer)) + d := NewDeployer(nil, WithExposer(exposer)) f := fn.Function{Name: "f", Expose: tt.expose, Deploy: fn.DeploySpec{Namespace: "ns"}} _, applied, err := d.resolveExposure(ctx, f, "ns", svc, clientset, dynClient, nil, nil) @@ -1041,7 +1041,7 @@ func Test_DeployNamespace(t *testing.T) { // domain or user labels there make later changes require recreation. Those // labels stay on object metadata and the pod template. func Test_DomainStaysOutOfSelectors(t *testing.T) { - d := NewDeployer() + d := NewDeployer(nil) // fake clientsets are passed explicitly below team := "red" teamKey := "team" f := fn.Function{ @@ -1104,7 +1104,7 @@ func Test_DomainStaysOutOfSelectors(t *testing.T) { // hold no such grant, so the Service create fails outright. KinD does not // run the plugin, so CI cannot catch a regression; this test can. func Test_generateService_OwnerReferenceOmitsBlockOwnerDeletion(t *testing.T) { - d := NewDeployer() + d := NewDeployer(nil) // fake clientsets are passed explicitly below f := fn.Function{Name: "f", Deploy: fn.DeploySpec{Image: "registry.example.com/f:latest"}} deployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "f", Namespace: "ns", UID: "uid-1"}} diff --git a/pkg/k8s/describer.go b/pkg/k8s/describer.go index d7a904e218..0b8d06dc0b 100644 --- a/pkg/k8s/describer.go +++ b/pkg/k8s/describer.go @@ -14,6 +14,7 @@ import ( ) type Describer struct { + kc *Client verbose bool transport http.RoundTripper } @@ -26,8 +27,8 @@ func WithDescriberTransport(transport http.RoundTripper) DescriberOpt { } } -func NewDescriber(verbose bool, opts ...DescriberOpt) *Describer { - d := &Describer{verbose: verbose} +func NewDescriber(kc *Client, verbose bool, opts ...DescriberOpt) *Describer { + d := &Describer{kc: kc, verbose: verbose} for _, o := range opts { o(d) } @@ -39,8 +40,11 @@ func (d *Describer) Describe(ctx context.Context, name, namespace string) (fn.In if namespace == "" { return fn.Instance{}, fmt.Errorf("function namespace is required when describing %q", name) } + if d.kc == nil { + return fn.Instance{}, fmt.Errorf("kubernetes client is not initialized") + } - clientset, err := NewKubernetesClientset() + clientset, err := d.kc.Clientset() if err != nil { return fn.Instance{}, fmt.Errorf("unable to create k8s client: %v", err) } diff --git a/pkg/k8s/describer_int_test.go b/pkg/k8s/describer_int_test.go index 2643b41359..264477ea4e 100644 --- a/pkg/k8s/describer_int_test.go +++ b/pkg/k8s/describer_int_test.go @@ -10,9 +10,10 @@ import ( ) func TestInt_Describe(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) describertesting.TestInt_Describe(t, - k8s.NewDescriber(true), - k8s.NewDeployer(k8s.WithDeployerVerbose(true)), - k8s.NewRemover(true), + k8s.NewDescriber(kc, true), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(true)), + k8s.NewRemover(kc, true), k8s.KubernetesDeployerName) } diff --git a/pkg/k8s/lister_int_test.go b/pkg/k8s/lister_int_test.go index 24747a80e6..633692bf84 100644 --- a/pkg/k8s/lister_int_test.go +++ b/pkg/k8s/lister_int_test.go @@ -13,8 +13,8 @@ func TestInt_List(t *testing.T) { kc := k8s.NewClient(k8s.GetClientConfig()) listertesting.TestInt_List(t, k8s.NewLister(kc, true), - k8s.NewDeployer(k8s.WithDeployerVerbose(true)), - k8s.NewDescriber(true), - k8s.NewRemover(true), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(true)), + k8s.NewDescriber(kc, true), + k8s.NewRemover(kc, true), k8s.KubernetesDeployerName) } diff --git a/pkg/k8s/openshift.go b/pkg/k8s/openshift.go index f47258618c..f417846485 100644 --- a/pkg/k8s/openshift.go +++ b/pkg/k8s/openshift.go @@ -25,8 +25,10 @@ const ( openShiftRegistryHostPort = openShiftRegistryHost + ":5000" ) -func GetOpenShiftServiceCA(ctx context.Context) (*x509.Certificate, error) { - client, ns, err := NewClientAndResolvedNamespace("") +// OpenShiftServiceCA fetches the OpenShift service CA certificate by creating +// a temporary ConfigMap annotated for CA bundle injection. +func (c *Client) OpenShiftServiceCA(ctx context.Context) (*x509.Certificate, error) { + client, ns, err := c.ClientAndNamespace("") if err != nil { return nil, err } @@ -86,6 +88,19 @@ func GetOpenShiftServiceCA(ctx context.Context) (*x509.Certificate, error) { } } +// DefaultOpenShiftRegistry returns the internal registry path for the active +// namespace. +func (c *Client) DefaultOpenShiftRegistry() string { + ns, _ := c.DefaultNamespace() + if ns == "" { + ns = "default" + } + + return openShiftRegistryHostPort + "/" + ns +} + +// GetDefaultOpenShiftRegistry reads the kubeconfig ad hoc. Remains only for +// callers not yet migrated to Client. Do not add new callers. func GetDefaultOpenShiftRegistry() string { ns, _ := GetDefaultNamespace() if ns == "" { @@ -101,10 +116,10 @@ func IsOpenShiftInternalRegistry(registry string) bool { return strings.HasPrefix(registry, openShiftRegistryHost) } -func GetOpenShiftDockerCredentialLoaders() []creds.CredentialsCallback { - conf := GetClientConfig() - - rawConf, err := conf.RawConfig() +// OpenShiftDockerCredentialLoaders returns a credential loader for the +// internal OpenShift registry, authenticating with the active user's token. +func (c *Client) OpenShiftDockerCredentialLoaders() []creds.CredentialsCallback { + rawConf, err := c.RawConfig() if err != nil { return nil } @@ -142,9 +157,12 @@ var ( detectErr error ) -// DetectOpenShift reports whether the cluster serves the OpenShift Route API. -// A non-nil error means the cluster could not be asked and the bool is -// meaningless. Probes once per process, answers from cache after. +// DetectOpenShift reads the kubeconfig ad hoc. Remains only for callers not +// yet migrated to Client. Do not add new callers; use (*Client).IsOpenShift. +// +// It reports whether the cluster serves the OpenShift Route API. A non-nil +// error means the cluster could not be asked and the bool is meaningless. +// Probes once per process, answers from cache after. func DetectOpenShift() (bool, error) { detectOnce.Do(func() { client, err := NewKubernetesClientset() diff --git a/pkg/k8s/remover.go b/pkg/k8s/remover.go index 563ef575c2..bccff1f097 100644 --- a/pkg/k8s/remover.go +++ b/pkg/k8s/remover.go @@ -10,13 +10,15 @@ import ( fn "knative.dev/func/pkg/functions" ) -func NewRemover(verbose bool) *Remover { +func NewRemover(kc *Client, verbose bool) *Remover { return &Remover{ + kc: kc, verbose: verbose, } } type Remover struct { + kc *Client verbose bool } @@ -25,8 +27,11 @@ func (remover *Remover) Remove(ctx context.Context, name, ns string) error { fmt.Fprintf(os.Stderr, "no namespace defined when trying to delete a function in knative remover\n") return fn.ErrNamespaceRequired } + if remover.kc == nil { + return fmt.Errorf("kubernetes client is not initialized") + } - clientset, err := NewKubernetesClientset() + clientset, err := remover.kc.Clientset() if err != nil { return fmt.Errorf("could not setup kubernetes clientset: %w", err) } diff --git a/pkg/k8s/remover_int_test.go b/pkg/k8s/remover_int_test.go index 5e4957c111..f1212a40f3 100644 --- a/pkg/k8s/remover_int_test.go +++ b/pkg/k8s/remover_int_test.go @@ -12,9 +12,9 @@ import ( func TestInt_Remove(t *testing.T) { kc := k8s.NewClient(k8s.GetClientConfig()) removertesting.TestInt_Remove(t, - k8s.NewRemover(true), - k8s.NewDeployer(k8s.WithDeployerVerbose(true)), - k8s.NewDescriber(true), + k8s.NewRemover(kc, true), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(true)), + k8s.NewDescriber(kc, true), k8s.NewLister(kc, true), k8s.KubernetesDeployerName) } diff --git a/pkg/keda/client.go b/pkg/keda/client.go index b39531281f..349e94776a 100644 --- a/pkg/keda/client.go +++ b/pkg/keda/client.go @@ -7,8 +7,11 @@ import ( "knative.dev/func/pkg/k8s" ) -func NewHTTPScaledObjectClientset() (*httpv1alpha1.Clientset, error) { - restConfig, err := k8s.GetClientConfig().ClientConfig() +func NewHTTPScaledObjectClientset(kc *k8s.Client) (*httpv1alpha1.Clientset, error) { + if kc == nil { + return nil, fmt.Errorf("kubernetes client is not initialized") + } + restConfig, err := kc.RestConfig() if err != nil { return nil, fmt.Errorf("failed to get clientconfig: %w", err) } diff --git a/pkg/keda/deployer.go b/pkg/keda/deployer.go index 86d268d2eb..c5b5d55403 100644 --- a/pkg/keda/deployer.go +++ b/pkg/keda/deployer.go @@ -30,12 +30,15 @@ type Deployer struct { verbose bool decorator deployer.DeployDecorator + kc *k8s.Client exposer deployer.Exposer } -func NewDeployer(opts ...DeployerOpt) *Deployer { +func NewDeployer(kc *k8s.Client, opts ...DeployerOpt) *Deployer { d := &Deployer{ + kc: kc, Deployer: *k8s.NewDeployer( + kc, // init with the kedaDeployerDecorator to have the correct deployer labels&annotations k8s.WithDeployerDecorator(&kedaDeployerDecorator{}), ), @@ -103,11 +106,14 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu return fn.DeploymentResult{}, err } - k8sClientset, err := k8s.NewKubernetesClientset() + if d.kc == nil { + return fn.DeploymentResult{}, fmt.Errorf("kubernetes client is not initialized") + } + k8sClientset, err := d.kc.Clientset() if err != nil { return fn.DeploymentResult{}, fmt.Errorf("failed to create K8sClientset: %v", err) } - dynClient, err := k8s.NewDynamicClient() + dynClient, err := d.kc.DynamicClient() if err != nil { return fn.DeploymentResult{}, fmt.Errorf("failed to create dynamic client: %w", err) } @@ -152,6 +158,7 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu minScale, maxScale := replicaBounds(f) target := deployTarget{ + kc: d.kc, clientset: k8sClientset, dynClient: dynClient, ref: ref, @@ -216,6 +223,7 @@ func (d *Deployer) validateExposure(f fn.Function, exposeRefusal error) error { // choosing a path: the clients, the function's placement, replica bounds, // and the live objects the HSO hangs off type deployTarget struct { + kc *k8s.Client clientset kubernetes.Interface dynClient dynamic.Interface ref deployer.ExposureRef @@ -456,7 +464,7 @@ func ensureHTTPScaledObject(ctx context.Context, t deployTarget, hosts []string) return fmt.Errorf("failed to generate http scaled object: %w", err) } - httpScaledObjectClientset, err := NewHTTPScaledObjectClientset() + httpScaledObjectClientset, err := NewHTTPScaledObjectClientset(t.kc) if err != nil { return fmt.Errorf("failed to create HTTPScaledObject clientset: %v", err) } diff --git a/pkg/keda/deployer_int_test.go b/pkg/keda/deployer_int_test.go index ca222e0a0b..448f4cacca 100644 --- a/pkg/keda/deployer_int_test.go +++ b/pkg/keda/deployer_int_test.go @@ -13,67 +13,74 @@ import ( func TestInt_FullPath(t *testing.T) { kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_FullPath(t, - keda.NewDeployer(keda.WithDeployerVerbose(false)), - keda.NewRemover(false), + keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), + keda.NewRemover(kc, false), keda.NewLister(kc, false), - keda.NewDescriber(false), + keda.NewDescriber(kc, false), keda.KedaDeployerName) } func TestInt_Deploy(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_Deploy(t, - keda.NewDeployer(keda.WithDeployerVerbose(false)), - keda.NewRemover(false), - keda.NewDescriber(false), + keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), + keda.NewRemover(kc, false), + keda.NewDescriber(kc, false), keda.KedaDeployerName) } func TestInt_Metadata(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_Metadata(t, - keda.NewDeployer(keda.WithDeployerVerbose(false)), - keda.NewRemover(false), - keda.NewDescriber(false), + keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), + keda.NewRemover(kc, false), + keda.NewDescriber(kc, false), keda.KedaDeployerName) } func TestInt_Events(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) t.Skip("Keda deployer does not support func subscribe yet") deployertesting.TestInt_Events(t, - keda.NewDeployer(keda.WithDeployerVerbose(false)), - keda.NewRemover(false), - keda.NewDescriber(false), + keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), + keda.NewRemover(kc, false), + keda.NewDescriber(kc, false), keda.KedaDeployerName) } func TestInt_Scale(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_Scale(t, - keda.NewDeployer(keda.WithDeployerVerbose(false)), - keda.NewRemover(false), - keda.NewDescriber(false), + keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), + keda.NewRemover(kc, false), + keda.NewDescriber(kc, false), keda.KedaDeployerName) } func TestInt_EnvsUpdate(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_EnvsUpdate(t, - keda.NewDeployer(keda.WithDeployerVerbose(false)), - keda.NewRemover(false), - keda.NewDescriber(false), + keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), + keda.NewRemover(kc, false), + keda.NewDescriber(kc, false), keda.KedaDeployerName) } func TestInt_ResourceValidationOnFirstDeploy(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_ResourceValidationOnFirstDeploy(t, - keda.NewDeployer(keda.WithDeployerVerbose(false)), - keda.NewRemover(false), - keda.NewDescriber(false), + keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), + keda.NewRemover(kc, false), + keda.NewDescriber(kc, false), keda.KedaDeployerName) } func TestInt_OperatorSync(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_OperatorSync(t, - keda.NewDeployer(keda.WithDeployerVerbose(false)), - keda.NewRemover(false), - keda.NewDescriber(false), + keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), + keda.NewRemover(kc, false), + keda.NewDescriber(kc, false), keda.KedaDeployerName) } diff --git a/pkg/keda/deployer_unit_test.go b/pkg/keda/deployer_unit_test.go index 54e6df6398..f7849714e5 100644 --- a/pkg/keda/deployer_unit_test.go +++ b/pkg/keda/deployer_unit_test.go @@ -80,7 +80,7 @@ func routeCount(t *testing.T, dynClient *dynamicfake.FakeDynamicClient) int { // client. func Test_RecordExposure_kedaRollback(t *testing.T) { routeName := interceptorExposureName(testFnName, testFnNS) - d := NewDeployer(WithExposer(ocproute.New(deployers.Keda))) + d := NewDeployer(nil, WithExposer(ocproute.New(deployers.Keda))) t.Run("record failure rolls the Route back", func(t *testing.T) { dynClient := newTestDynClient(kedaRoute(routeName, testInterceptorNS, testFnName, testFnNS)) @@ -143,7 +143,7 @@ func Test_RecordExposure_kedaRollback(t *testing.T) { // and a nil Exposer leaves everything alone. func Test_clearExposure(t *testing.T) { routeName := interceptorExposureName(testFnName, testFnNS) - d := NewDeployer(WithExposer(ocproute.New(deployers.Keda))) + d := NewDeployer(nil, WithExposer(ocproute.New(deployers.Keda))) t.Run("removes the recorded Route, then clears the record", func(t *testing.T) { dynClient := newTestDynClient(kedaRoute(routeName, testInterceptorNS, testFnName, testFnNS)) @@ -209,7 +209,7 @@ func Test_clearExposure(t *testing.T) { dynClient := newTestDynClient(kedaRoute(routeName, testInterceptorNS, testFnName, testFnNS)) clientset := newTestClientset(false, testRecord()) - if err := NewDeployer().clearExposure(t.Context(), newTestTarget(clientset, dynClient), testInterceptorNS); err != nil { + if err := NewDeployer(nil).clearExposure(t.Context(), newTestTarget(clientset, dynClient), testInterceptorNS); err != nil { t.Fatalf("expected a nil exposer to be a no-op, got: %v", err) } if n := len(dynClient.Actions()); n != 0 { @@ -230,7 +230,7 @@ func Test_clearExposure(t *testing.T) { // interceptor nobody asked to route through. func Test_validateExposure(t *testing.T) { refusal := fmt.Errorf("interceptor missing") - d := NewDeployer(WithExposer(ocproute.New(deployers.Keda))) + d := NewDeployer(nil, WithExposer(ocproute.New(deployers.Keda))) f := fn.Function{Name: testFnName, Namespace: testFnNS, Expose: fn.ExposeRoute} if err := d.validateExposure(f, refusal); err == nil || !strings.Contains(err.Error(), "cannot expose") { @@ -247,7 +247,7 @@ func Test_validateExposure(t *testing.T) { } f.Expose = fn.ExposeRoute - if err := NewDeployer().validateExposure(f, refusal); err != nil { + if err := NewDeployer(nil).validateExposure(f, refusal); err != nil { t.Errorf("expected a nil exposer to skip exposure validation, got: %v", err) } } diff --git a/pkg/keda/describer.go b/pkg/keda/describer.go index b1fb36e132..5d9dd3e5c8 100644 --- a/pkg/keda/describer.go +++ b/pkg/keda/describer.go @@ -16,6 +16,7 @@ import ( ) type Describer struct { + kc *k8s.Client verbose bool transport http.RoundTripper } @@ -28,8 +29,8 @@ func WithDescriberTransport(transport http.RoundTripper) DescriberOpt { } } -func NewDescriber(verbose bool, opts ...DescriberOpt) *Describer { - d := &Describer{verbose: verbose} +func NewDescriber(kc *k8s.Client, verbose bool, opts ...DescriberOpt) *Describer { + d := &Describer{kc: kc, verbose: verbose} for _, o := range opts { o(d) } @@ -41,8 +42,11 @@ func (d *Describer) Describe(ctx context.Context, name, namespace string) (fn.In if namespace == "" { return fn.Instance{}, fmt.Errorf("function namespace is required when describing %q", name) } + if d.kc == nil { + return fn.Instance{}, fmt.Errorf("kubernetes client is not initialized") + } - clientset, err := k8s.NewKubernetesClientset() + clientset, err := d.kc.Clientset() if err != nil { return fn.Instance{}, fmt.Errorf("unable to create k8s client: %v", err) } @@ -66,7 +70,7 @@ func (d *Describer) Describe(ctx context.Context, name, namespace string) (fn.In // We're responsible, for this function --> proceed... - httpScaledObjectClientset, err := NewHTTPScaledObjectClientset() + httpScaledObjectClientset, err := NewHTTPScaledObjectClientset(d.kc) if err != nil { return fn.Instance{}, fmt.Errorf("unable to create HTTPScaledObject client: %v", err) } diff --git a/pkg/keda/describer_int_test.go b/pkg/keda/describer_int_test.go index bc32ccdd70..60a2271268 100644 --- a/pkg/keda/describer_int_test.go +++ b/pkg/keda/describer_int_test.go @@ -6,13 +6,15 @@ import ( "testing" describertesting "knative.dev/func/pkg/describer/testing" + "knative.dev/func/pkg/k8s" keda "knative.dev/func/pkg/keda" ) func TestInt_Describe(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) describertesting.TestInt_Describe(t, - keda.NewDescriber(true), - keda.NewDeployer(keda.WithDeployerVerbose(true)), - keda.NewRemover(true), + keda.NewDescriber(kc, true), + keda.NewDeployer(kc, keda.WithDeployerVerbose(true)), + keda.NewRemover(kc, true), keda.KedaDeployerName) } diff --git a/pkg/keda/lister.go b/pkg/keda/lister.go index 7e0edd0c9e..b7f1e7607d 100644 --- a/pkg/keda/lister.go +++ b/pkg/keda/lister.go @@ -36,7 +36,7 @@ func (l *Lister) List(ctx context.Context, namespace string) ([]fn.ListItem, err return nil, fmt.Errorf("unable to create k8s client: %v", err) } - restConfig, err := l.kc.ClientConfig() + restConfig, err := l.kc.RestConfig() if err != nil { return nil, fmt.Errorf("unable to get kubernetes client config: %v", err) } diff --git a/pkg/keda/lister_int_test.go b/pkg/keda/lister_int_test.go index 5fe04ed664..955da22e8a 100644 --- a/pkg/keda/lister_int_test.go +++ b/pkg/keda/lister_int_test.go @@ -14,8 +14,8 @@ func TestInt_List(t *testing.T) { kc := k8s.NewClient(k8s.GetClientConfig()) listertesting.TestInt_List(t, keda.NewLister(kc, true), - keda.NewDeployer(keda.WithDeployerVerbose(true)), - keda.NewDescriber(true), - keda.NewRemover(true), + keda.NewDeployer(kc, keda.WithDeployerVerbose(true)), + keda.NewDescriber(kc, true), + keda.NewRemover(kc, true), keda.KedaDeployerName) } diff --git a/pkg/keda/remover.go b/pkg/keda/remover.go index 21e5955a84..3bb3fda2de 100644 --- a/pkg/keda/remover.go +++ b/pkg/keda/remover.go @@ -13,13 +13,15 @@ import ( "knative.dev/func/pkg/ocproute" ) -func NewRemover(verbose bool) *Remover { +func NewRemover(kc *k8s.Client, verbose bool) *Remover { return &Remover{ + kc: kc, verbose: verbose, } } type Remover struct { + kc *k8s.Client verbose bool } @@ -28,8 +30,11 @@ func (remover *Remover) Remove(ctx context.Context, name, ns string) error { fmt.Fprintf(os.Stderr, "no namespace defined when trying to delete a function in keda remover\n") return fn.ErrNamespaceRequired } + if remover.kc == nil { + return fmt.Errorf("kubernetes client is not initialized") + } - clientset, err := k8s.NewKubernetesClientset() + clientset, err := remover.kc.Clientset() if err != nil { return fmt.Errorf("could not setup kubernetes clientset: %w", err) } diff --git a/pkg/keda/remover_int_test.go b/pkg/keda/remover_int_test.go index 9b658e607a..8d406754c2 100644 --- a/pkg/keda/remover_int_test.go +++ b/pkg/keda/remover_int_test.go @@ -13,9 +13,9 @@ import ( func TestInt_Remove(t *testing.T) { kc := k8s.NewClient(k8s.GetClientConfig()) removertesting.TestInt_Remove(t, - keda.NewRemover(true), - keda.NewDeployer(keda.WithDeployerVerbose(true)), - keda.NewDescriber(true), + keda.NewRemover(kc, true), + keda.NewDeployer(kc, keda.WithDeployerVerbose(true)), + keda.NewDescriber(kc, true), keda.NewLister(kc, true), keda.KedaDeployerName) } diff --git a/pkg/knative/client.go b/pkg/knative/client.go index 170213b68b..171defd1d1 100644 --- a/pkg/knative/client.go +++ b/pkg/knative/client.go @@ -14,12 +14,15 @@ import ( "knative.dev/func/pkg/k8s" ) -func NewServingClient(namespace string) (clientservingv1.KnServingClient, error) { +func NewServingClient(kc *k8s.Client, namespace string) (clientservingv1.KnServingClient, error) { if err := validateKubeconfigFile(); err != nil { return nil, err } + if kc == nil { + return nil, fmt.Errorf("kubernetes client is not initialized") + } - restConfig, err := k8s.GetClientConfig().ClientConfig() + restConfig, err := kc.RestConfig() if err != nil { return nil, fmt.Errorf("failed to create new serving client: %v", err) } @@ -34,12 +37,15 @@ func NewServingClient(namespace string) (clientservingv1.KnServingClient, error) return client, nil } -func NewEventingClient(namespace string) (clienteventingv1.KnEventingClient, error) { +func NewEventingClient(kc *k8s.Client, namespace string) (clienteventingv1.KnEventingClient, error) { if err := validateKubeconfigFile(); err != nil { return nil, err } + if kc == nil { + return nil, fmt.Errorf("kubernetes client is not initialized") + } - restConfig, err := k8s.GetClientConfig().ClientConfig() + restConfig, err := kc.RestConfig() if err != nil { return nil, fmt.Errorf("failed to create new eventing client: %v", err) } diff --git a/pkg/knative/deployer.go b/pkg/knative/deployer.go index 6027c46126..4aa7e46401 100644 --- a/pkg/knative/deployer.go +++ b/pkg/knative/deployer.go @@ -53,10 +53,12 @@ type Deployer struct { verbose bool decorator deployer.DeployDecorator + + kc *k8s.Client } -func NewDeployer(opts ...DeployerOpt) *Deployer { - d := &Deployer{} +func NewDeployer(kc *k8s.Client, opts ...DeployerOpt) *Deployer { + d := &Deployer{kc: kc} for _, opt := range opts { opt(d) @@ -84,7 +86,7 @@ func (d *Deployer) isImageInPrivateRegistry(ctx context.Context, client clientse if err != nil { return false } - k8sClient, err := k8s.NewKubernetesClientset() + k8sClient, err := d.kc.Clientset() if err != nil { return false } @@ -107,7 +109,7 @@ func (d *Deployer) isImageInPrivateRegistry(ctx context.Context, client clientse return false } -func onClusterFix(f fn.Function) fn.Function { +func (d *Deployer) onClusterFix(f fn.Function) fn.Function { // This only exists because of a bootstrapping problem with On-Cluster // builds: It appears that, when sending a function to be built on-cluster // the target namespace is not being transmitted in the pipeline @@ -116,13 +118,16 @@ func onClusterFix(f fn.Function) fn.Function { // earlier versions of this logic relied entirely on the current // kubernetes context. if f.Namespace == "" && f.Deploy.Namespace == "" { - f.Namespace, _ = k8s.GetDefaultNamespace() + f.Namespace, _ = d.kc.DefaultNamespace() } return f } func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResult, error) { - f = onClusterFix(f) + if d.kc == nil { + return fn.DeploymentResult{}, fmt.Errorf("kubernetes client is not initialized") + } + f = d.onClusterFix(f) // Choosing f.Namespace vs f.Deploy.Namespace: // This is minimal logic currently required of all deployer impls. // If f.Namespace is defined, this is the (possibly new) target @@ -150,18 +155,19 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu f.Deploy.Image = f.Build.Image } + kc := d.kc // Clients - client, err := NewServingClient(namespace) + client, err := NewServingClient(kc, namespace) if err != nil { return fn.DeploymentResult{}, wrapDeployerClientError(err) } - eventingClient, err := NewEventingClient(namespace) + eventingClient, err := NewEventingClient(kc, namespace) if err != nil { return fn.DeploymentResult{}, wrapDeployerClientError(err) } // check if 'dapr-system' namespace exists daprInstalled := false - k8sClient, err := k8s.NewKubernetesClientset() + k8sClient, err := kc.Clientset() if err != nil { return fn.DeploymentResult{}, wrapDeployerClientError(err) } @@ -170,7 +176,11 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu daprInstalled = true } - t := fnhttp.NewRoundTripper(fnhttp.WithOpenShiftServiceCA(), fnhttp.WithInsecureSkipVerify(f.RegistryInsecure)) + t := fnhttp.NewRoundTripper( + fnhttp.WithOpenShiftServiceCA(kc), + fnhttp.WithInsecureSkipVerify(f.RegistryInsecure), + fnhttp.WithInClusterDialer(k8s.NewLazyInitInClusterDialer(kc.Loader())), + ) defer func(t fnhttp.RoundTripCloser) { _ = t.Close() }(t) diff --git a/pkg/knative/deployer_int_test.go b/pkg/knative/deployer_int_test.go index 3d3801d0e4..9b8ea38d2f 100644 --- a/pkg/knative/deployer_int_test.go +++ b/pkg/knative/deployer_int_test.go @@ -13,65 +13,72 @@ import ( func TestInt_FullPath(t *testing.T) { kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_FullPath(t, - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewRemover(true), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewRemover(kc, true), knative.NewLister(kc, true), - knative.NewDescriber(true), + knative.NewDescriber(kc, true), knative.KnativeDeployerName) } func TestInt_Deploy(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_Deploy(t, - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewRemover(false), - knative.NewDescriber(false), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewRemover(kc, false), + knative.NewDescriber(kc, false), knative.KnativeDeployerName) } func TestInt_Metadata(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_Metadata(t, - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewRemover(false), - knative.NewDescriber(false), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewRemover(kc, false), + knative.NewDescriber(kc, false), knative.KnativeDeployerName) } func TestInt_Events(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_Events(t, - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewRemover(false), - knative.NewDescriber(false), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewRemover(kc, false), + knative.NewDescriber(kc, false), knative.KnativeDeployerName) } func TestInt_Scale(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_Scale(t, - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewRemover(false), - knative.NewDescriber(false), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewRemover(kc, false), + knative.NewDescriber(kc, false), knative.KnativeDeployerName) } func TestInt_EnvsUpdate(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_EnvsUpdate(t, - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewRemover(false), - knative.NewDescriber(false), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewRemover(kc, false), + knative.NewDescriber(kc, false), knative.KnativeDeployerName) } func TestInt_ResourceValidationOnFirstDeploy(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_ResourceValidationOnFirstDeploy(t, - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewRemover(false), - knative.NewDescriber(false), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewRemover(kc, false), + knative.NewDescriber(kc, false), knative.KnativeDeployerName) } func TestInt_OperatorSync(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) deployertesting.TestInt_OperatorSync(t, - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewRemover(false), - knative.NewDescriber(false), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewRemover(kc, false), + knative.NewDescriber(kc, false), knative.KnativeDeployerName) } diff --git a/pkg/knative/describer.go b/pkg/knative/describer.go index e2b42d9c1d..5218353ed1 100644 --- a/pkg/knative/describer.go +++ b/pkg/knative/describer.go @@ -18,6 +18,7 @@ import ( ) type Describer struct { + kc *k8s.Client verbose bool transport http.RoundTripper } @@ -30,8 +31,8 @@ func WithDescriberTransport(transport http.RoundTripper) DescriberOpt { } } -func NewDescriber(verbose bool, opts ...DescriberOpt) *Describer { - d := &Describer{verbose: verbose} +func NewDescriber(kc *k8s.Client, verbose bool, opts ...DescriberOpt) *Describer { + d := &Describer{kc: kc, verbose: verbose} for _, o := range opts { o(d) } @@ -47,13 +48,16 @@ func (d *Describer) Describe(ctx context.Context, name, namespace string) (fn.In if namespace == "" { return fn.Instance{}, fmt.Errorf("function namespace is required when describing %q", name) } + if d.kc == nil { + return fn.Instance{}, fmt.Errorf("kubernetes client is not initialized") + } - servingClient, err := NewServingClient(namespace) + servingClient, err := NewServingClient(d.kc, namespace) if err != nil { return fn.Instance{}, err } - eventingClient, err := NewEventingClient(namespace) + eventingClient, err := NewEventingClient(d.kc, namespace) if err != nil { return fn.Instance{}, err } @@ -124,7 +128,7 @@ func (d *Describer) Describe(ctx context.Context, name, namespace string) (fn.In } // get used image (including the sha) - clientset, err := k8s.NewKubernetesClientset() + clientset, err := d.kc.Clientset() if err != nil { return fn.Instance{}, fmt.Errorf("unable to create k8s client: %v", err) } diff --git a/pkg/knative/describer_int_test.go b/pkg/knative/describer_int_test.go index 4ddc036488..b0484b310b 100644 --- a/pkg/knative/describer_int_test.go +++ b/pkg/knative/describer_int_test.go @@ -6,13 +6,15 @@ import ( "testing" describertesting "knative.dev/func/pkg/describer/testing" + "knative.dev/func/pkg/k8s" "knative.dev/func/pkg/knative" ) func TestInt_Describe(t *testing.T) { + kc := k8s.NewClient(k8s.GetClientConfig()) describertesting.TestInt_Describe(t, - knative.NewDescriber(true), - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewRemover(true), + knative.NewDescriber(kc, true), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewRemover(kc, true), knative.KnativeDeployerName) } diff --git a/pkg/knative/lister.go b/pkg/knative/lister.go index feb76ba6bc..439534509e 100644 --- a/pkg/knative/lister.go +++ b/pkg/knative/lister.go @@ -32,7 +32,7 @@ func (l *Lister) List(ctx context.Context, namespace string) ([]fn.ListItem, err return nil, fmt.Errorf("kubernetes client is not initialized") } - restConfig, err := l.kc.ClientConfig() + restConfig, err := l.kc.RestConfig() if err != nil { return nil, fmt.Errorf("unable to get kubernetes client config: %w", err) } diff --git a/pkg/knative/lister_int_test.go b/pkg/knative/lister_int_test.go index b379b34b14..9078fda08c 100644 --- a/pkg/knative/lister_int_test.go +++ b/pkg/knative/lister_int_test.go @@ -14,8 +14,8 @@ func TestInt_List(t *testing.T) { kc := k8s.NewClient(k8s.GetClientConfig()) listertesting.TestInt_List(t, knative.NewLister(kc, true), - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewDescriber(true), - knative.NewRemover(true), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewDescriber(kc, true), + knative.NewRemover(kc, true), knative.KnativeDeployerName) } diff --git a/pkg/knative/remover.go b/pkg/knative/remover.go index 372e1a7776..0a8a1169fd 100644 --- a/pkg/knative/remover.go +++ b/pkg/knative/remover.go @@ -8,17 +8,20 @@ import ( apiErrors "k8s.io/apimachinery/pkg/api/errors" fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" ) const RemoveTimeout = 120 * time.Second -func NewRemover(verbose bool) *Remover { +func NewRemover(kc *k8s.Client, verbose bool) *Remover { return &Remover{ + kc: kc, verbose: verbose, } } type Remover struct { + kc *k8s.Client verbose bool } @@ -27,8 +30,11 @@ func (remover *Remover) Remove(ctx context.Context, name, ns string) error { fmt.Fprintf(os.Stderr, "no namespace defined when trying to delete a function in knative remover\n") return fn.ErrNamespaceRequired } + if remover.kc == nil { + return fmt.Errorf("kubernetes client is not initialized") + } - client, err := NewServingClient(ns) + client, err := NewServingClient(remover.kc, ns) if err != nil { return err } diff --git a/pkg/knative/remover_int_test.go b/pkg/knative/remover_int_test.go index 325f035034..d03ede4845 100644 --- a/pkg/knative/remover_int_test.go +++ b/pkg/knative/remover_int_test.go @@ -13,9 +13,9 @@ import ( func TestInt_Remove(t *testing.T) { kc := k8s.NewClient(k8s.GetClientConfig()) removertesting.TestInt_Remove(t, - knative.NewRemover(true), - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewDescriber(true), + knative.NewRemover(kc, true), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewDescriber(kc, true), knative.NewLister(kc, true), knative.KnativeDeployerName) } diff --git a/pkg/pipelines/tekton/pipelines_int_test.go b/pkg/pipelines/tekton/pipelines_int_test.go index 52bb3dac5a..ba13781302 100644 --- a/pkg/pipelines/tekton/pipelines_int_test.go +++ b/pkg/pipelines/tekton/pipelines_int_test.go @@ -49,19 +49,19 @@ func newRemoteTestClient(verbose bool, deployer string, opts ...fn.Option) *fn.C baseOpts := []fn.Option{ fn.WithBuilder(buildpacks.NewBuilder(buildpacks.WithVerbose(verbose))), fn.WithPusher(docker.NewPusher(docker.WithCredentialsProvider(testCP))), - fn.WithDescribers(knative.NewDescriber(verbose), k8s.NewDescriber(verbose), keda.NewDescriber(verbose)), + fn.WithDescribers(knative.NewDescriber(kc, verbose), k8s.NewDescriber(kc, verbose), keda.NewDescriber(kc, verbose)), fn.WithListers(knative.NewLister(kc, verbose), k8s.NewLister(kc, verbose), keda.NewLister(kc, verbose)), - fn.WithRemovers(knative.NewRemover(verbose), k8s.NewRemover(verbose), keda.NewRemover(verbose)), - fn.WithPipelinesProvider(tekton.NewPipelinesProvider(tekton.WithCredentialsProvider(testCP), tekton.WithVerbose(verbose))), + fn.WithRemovers(knative.NewRemover(kc, verbose), k8s.NewRemover(kc, verbose), keda.NewRemover(kc, verbose)), + fn.WithPipelinesProvider(tekton.NewPipelinesProvider(tekton.WithCredentialsProvider(testCP), tekton.WithVerbose(verbose), tekton.WithK8sClient(kc))), } switch deployer { case k8s.KubernetesDeployerName: - baseOpts = append(baseOpts, fn.WithDeployer(k8s.NewDeployer(k8s.WithDeployerVerbose(verbose)))) + baseOpts = append(baseOpts, fn.WithDeployer(k8s.NewDeployer(kc, k8s.WithDeployerVerbose(verbose)))) case keda.KedaDeployerName: - baseOpts = append(baseOpts, fn.WithDeployer(keda.NewDeployer(keda.WithDeployerVerbose(verbose)))) + baseOpts = append(baseOpts, fn.WithDeployer(keda.NewDeployer(kc, keda.WithDeployerVerbose(verbose)))) case knative.KnativeDeployerName: - baseOpts = append(baseOpts, fn.WithDeployer(knative.NewDeployer(knative.WithDeployerVerbose(verbose)))) + baseOpts = append(baseOpts, fn.WithDeployer(knative.NewDeployer(kc, knative.WithDeployerVerbose(verbose)))) } return fn.New(append(baseOpts, opts...)...) diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index ef937b29a8..f384c60112 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -58,6 +58,7 @@ type PipelinesProvider struct { credentialsProvider oci.CredentialsProvider decorator PipelineDecorator transport http.RoundTripper + kc *k8s.Client } func WithCredentialsProvider(credentialsProvider oci.CredentialsProvider) Opt { @@ -84,6 +85,12 @@ func WithTransport(transport http.RoundTripper) Opt { } } +func WithK8sClient(kc *k8s.Client) Opt { + return func(pp *PipelinesProvider) { + pp.kc = kc + } +} + func WithPacURLCallback(getPacURL pacURLCallback) Opt { return func(pp *PipelinesProvider) { pp.getPacURL = getPacURL @@ -269,12 +276,12 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn var describer fn.Describer switch f.Deploy.Deployer { case k8s.KubernetesDeployerName: - describer = k8s.NewDescriber(false, k8s.WithDescriberTransport(pp.transport)) + describer = k8s.NewDescriber(pp.kc, false, k8s.WithDescriberTransport(pp.transport)) case keda.KedaDeployerName: - describer = keda.NewDescriber(false, keda.WithDescriberTransport(pp.transport)) + describer = keda.NewDescriber(pp.kc, false, keda.WithDescriberTransport(pp.transport)) default: // default to knative - describer = knative.NewDescriber(false, knative.WithDescriberTransport(pp.transport)) + describer = knative.NewDescriber(pp.kc, false, knative.WithDescriberTransport(pp.transport)) } obj, err := describer.Describe(ctx, f.Name, f.Namespace) diff --git a/pkg/testing/testing.go b/pkg/testing/testing.go index e006b0265b..d02ebf0c9e 100644 --- a/pkg/testing/testing.go +++ b/pkg/testing/testing.go @@ -22,12 +22,16 @@ import ( "net" "net/http" "net/http/cgi" + "net/http/httptest" "os" "os/exec" "path/filepath" "runtime" "strings" "testing" + + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) const DefaultIntTestRegistry = "registry.localtest.me/func" @@ -331,3 +335,62 @@ func Registry() string { // Default to localhost registry (same as E2E tests) return DefaultIntTestRegistry } + +// FakeCluster starts a fake Kubernetes API server which answers the +// discovery request used for OpenShift detection and nothing else, writes a +// kubeconfig pointing at it and sets KUBECONFIG to that file for the test. +// The server reports itself as OpenShift when openshift is true. +// It returns the kubeconfig path. +func FakeCluster(t *testing.T, openshift bool) string { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if openshift && r.URL.Path == "/apis/route.openshift.io/v1" { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"kind":"APIResourceList","apiVersion":"v1","groupVersion":"route.openshift.io/v1","resources":[]}`)) + return + } + http.NotFound(w, r) + })) + t.Cleanup(srv.Close) + + cfg := clientcmdapi.Config{ + CurrentContext: "fake", + Contexts: map[string]*clientcmdapi.Context{"fake": {Cluster: "fake", AuthInfo: "fake", Namespace: "default"}}, + Clusters: map[string]*clientcmdapi.Cluster{"fake": {Server: srv.URL}}, + AuthInfos: map[string]*clientcmdapi.AuthInfo{"fake": {Token: "fake-token"}}, + } + path := filepath.Join(t.TempDir(), "kubeconfig") + if err := clientcmd.WriteToFile(cfg, path); err != nil { + t.Fatal(err) + } + t.Setenv("KUBECONFIG", path) + return path +} + +// UnreachableCluster points KUBECONFIG at a kubeconfig whose server is a +// closed local port, so every API call fails with a connection error. Use it +// to test code that must tell "could not ask the cluster" apart from a +// cluster's answer. Returns the kubeconfig path. +func UnreachableCluster(t *testing.T) string { + t.Helper() + // Bind and immediately close a port so nothing listens on it. + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + server := "https://" + l.Addr().String() + _ = l.Close() + + cfg := clientcmdapi.Config{ + CurrentContext: "unreachable", + Contexts: map[string]*clientcmdapi.Context{"unreachable": {Cluster: "unreachable", AuthInfo: "unreachable", Namespace: "default"}}, + Clusters: map[string]*clientcmdapi.Cluster{"unreachable": {Server: server, InsecureSkipTLSVerify: true}}, + AuthInfos: map[string]*clientcmdapi.AuthInfo{"unreachable": {Token: "unreachable-token"}}, + } + path := filepath.Join(t.TempDir(), "kubeconfig") + if err := clientcmd.WriteToFile(cfg, path); err != nil { + t.Fatal(err) + } + t.Setenv("KUBECONFIG", path) + return path +} From 7284bda0d4c7d709c4515be008630db18f600aa7 Mon Sep 17 00:00:00 2001 From: gauron99 Date: Thu, 27 Aug 2026 21:15:15 +0200 Subject: [PATCH 2/3] Inject k8s.Client into pkg/k8s helpers, dialer and transport Every cluster-facing helper in pkg/k8s now takes the *Client as its first argument after ctx: secrets, configmaps, persistent volumes, pod logs, service accounts, manifestival, CheckResourcesArePresent. The in-cluster dialer and fnhttp.NewRoundTripper are built from the Client instead of a clientcmd.ClientConfig, and the pod security contexts take the OpenShift flag from the Client rather than a package global. Callers pass the Client they already hold (deployers, tekton PipelinesProvider) or resolve it at the command (config envs/volumes, logs). The operator syncer and tekton clients still resolve from the kubeconfig themselves; they are marked TODO for the next step. SetOpenShiftForTest is gone: the security context tests pass the flag directly. The shared rest.Config is copied before the dialer and volume upload mutate it. --- cmd/client.go | 4 +- cmd/config.go | 5 ++- cmd/config_envs.go | 8 ++-- cmd/config_volumes.go | 10 ++--- cmd/logs.go | 4 +- .../testing/integration_test_helper.go | 7 ++-- pkg/http/openshift_int_test.go | 2 +- pkg/http/transport.go | 6 ++- pkg/http/transport_test.go | 2 +- pkg/k8s/configmaps.go | 12 +++--- pkg/k8s/configmaps_test.go | 4 +- pkg/k8s/deployer.go | 16 ++++---- pkg/k8s/dialer.go | 37 ++++++++++--------- pkg/k8s/dialer_int_test.go | 6 +-- pkg/k8s/logs.go | 20 +++++++--- pkg/k8s/logs_int_test.go | 6 +-- pkg/k8s/manifestival.go | 4 +- pkg/k8s/persistent_volumes.go | 35 +++++++++--------- pkg/k8s/persistent_volumes_int_test.go | 15 ++++---- pkg/k8s/secrets.go | 26 ++++++------- pkg/k8s/secrets_test.go | 4 +- pkg/k8s/security_context.go | 4 +- pkg/k8s/security_context_test.go | 19 ++-------- pkg/k8s/serviceaccount.go | 4 +- pkg/knative/deployer.go | 10 ++--- pkg/knative/logs.go | 4 +- pkg/operator/sync.go | 11 ++++-- pkg/operator/sync_test.go | 15 ++++---- pkg/pipelines/tekton/pipelines_int_test.go | 2 +- .../tekton/pipelines_pac_provider.go | 10 ++--- pkg/pipelines/tekton/pipelines_provider.go | 28 ++++++++------ .../tekton/pipelines_provider_test.go | 11 +++--- pkg/pipelines/tekton/resources_pac.go | 4 +- pkg/pipelines/tekton/templates.go | 16 ++++---- pkg/pipelines/tekton/templates_int_test.go | 5 ++- pkg/pipelines/tekton/templates_test.go | 9 +++-- 36 files changed, 200 insertions(+), 185 deletions(-) diff --git a/cmd/client.go b/cmd/client.go index 9b6ab5cfad..f6562b87a4 100644 --- a/cmd/client.go +++ b/cmd/client.go @@ -129,9 +129,9 @@ func newTransport(kc *k8s.Client, insecureSkipVerify bool) fnhttp.RoundTripClose fnhttp.WithOpenShiftServiceCA(kc), } if kc != nil && kc.Loader() != nil { - opts = append(opts, fnhttp.WithInClusterDialer(k8s.NewLazyInitInClusterDialer(kc.Loader()))) + opts = append(opts, fnhttp.WithInClusterDialer(k8s.NewLazyInitInClusterDialer(kc))) } - return fnhttp.NewRoundTripper(opts...) + return fnhttp.NewRoundTripper(kc, opts...) } // newCredentialsProvider returns a credentials provider which possibly diff --git a/cmd/config.go b/cmd/config.go index f5e1615728..e35a85c46a 100644 --- a/cmd/config.go +++ b/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": diff --git a/cmd/config_envs.go b/cmd/config_envs.go index 4d03a0f347..0ae3ef4e1b 100644 --- a/cmd/config_envs.go +++ b/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 } diff --git a/cmd/config_volumes.go b/cmd/config_volumes.go index c0b3926351..f6e5bf1288 100644 --- a/cmd/config_volumes.go +++ b/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 } diff --git a/cmd/logs.go b/cmd/logs.go index b5207240c9..e75bd940e4 100644 --- a/cmd/logs.go +++ b/cmd/logs.go @@ -23,7 +23,9 @@ import ( type logGatherer func(context.Context, knative.LogsOptions, io.Writer) error func NewLogsCmd(newClient ClientFactory) *cobra.Command { - return newLogsCmd(newClient, knative.GetKServiceLogs) + return newLogsCmd(newClient, func(ctx context.Context, opts knative.LogsOptions, out io.Writer) error { + return knative.GetKServiceLogs(ctx, k8s.NewClientFromKubeconfig(), opts, out) + }) } // newLogsCmd constructs the command with an explicit log gatherer, allowing diff --git a/pkg/deployer/testing/integration_test_helper.go b/pkg/deployer/testing/integration_test_helper.go index 42f7478879..4cefbfa5ce 100644 --- a/pkg/deployer/testing/integration_test_helper.go +++ b/pkg/deployer/testing/integration_test_helper.go @@ -768,7 +768,7 @@ func TestInt_FullPath(t *testing.T, deployer fn.Deployer, remover fn.Remover, li buff := new(k8s.SynchronizedBuffer) go func() { selector := fmt.Sprintf("function.knative.dev/name=%s", functionName) - _ = k8s.GetPodLogsBySelector(ctx, k8s.PodLogsOptions{ + _ = k8s.GetPodLogsBySelector(ctx, k8s.NewClientFromKubeconfig(), k8s.PodLogsOptions{ Namespace: namespace, LabelSelector: selector, Container: "user-container", @@ -854,7 +854,7 @@ func TestInt_FullPath(t *testing.T, deployer fn.Deployer, remover fn.Remover, li redeployLogBuff := new(k8s.SynchronizedBuffer) go func() { selector := fmt.Sprintf("function.knative.dev/name=%s", functionName) - _ = k8s.GetPodLogsBySelector(ctx, k8s.PodLogsOptions{ + _ = k8s.GetPodLogsBySelector(ctx, k8s.NewClientFromKubeconfig(), k8s.PodLogsOptions{ Namespace: namespace, LabelSelector: selector, Container: "user-container", @@ -1419,8 +1419,7 @@ func getHttpClient(ctx context.Context, deployer string) (*http.Client, func(), case k8s.KubernetesDeployerName, keda.KedaDeployerName: // For Kubernetes deployments, use in-cluster dialer to access ClusterIP services - clientConfig := k8s.GetClientConfig() - dialer, err := k8s.NewInClusterDialer(ctx, clientConfig) + dialer, err := k8s.NewInClusterDialer(ctx, k8s.NewClientFromKubeconfig()) if err != nil { return nil, noopDeferFunc, fmt.Errorf("failed to create in-cluster dialer: %w", err) } diff --git a/pkg/http/openshift_int_test.go b/pkg/http/openshift_int_test.go index b26f19ada7..795569aa6f 100644 --- a/pkg/http/openshift_int_test.go +++ b/pkg/http/openshift_int_test.go @@ -17,7 +17,7 @@ func TestInt_RoundTripper(t *testing.T) { return } - transport := fnhttp.NewRoundTripper(fnhttp.WithOpenShiftServiceCA(kc)) + transport := fnhttp.NewRoundTripper(kc, fnhttp.WithOpenShiftServiceCA(kc)) defer transport.Close() client := http.Client{ diff --git a/pkg/http/transport.go b/pkg/http/transport.go index f651f1d1ce..8947ed8cc6 100644 --- a/pkg/http/transport.go +++ b/pkg/http/transport.go @@ -56,14 +56,16 @@ func WithInsecureSkipVerify(insecureSkipVerify bool) Option { // // This is useful for accessing cluster internal services (pushing a CloudEvent // into Knative broker). -func NewRoundTripper(opts ...Option) RoundTripCloser { +func NewRoundTripper(kc *k8s.Client, opts ...Option) RoundTripCloser { o := options{ - inClusterDialer: k8s.NewLazyInitInClusterDialer(k8s.GetClientConfig()), insecureSkipVerify: false, } for _, option := range opts { option(&o) } + if o.inClusterDialer == nil { + o.inClusterDialer = k8s.NewLazyInitInClusterDialer(kc) + } httpTransport := newHTTPTransport() diff --git a/pkg/http/transport_test.go b/pkg/http/transport_test.go index ee6b9e744f..8d02db76b4 100644 --- a/pkg/http/transport_test.go +++ b/pkg/http/transport_test.go @@ -39,7 +39,7 @@ func TestCustomCA(t *testing.T) { backingAddr: inClusterAddr, } - tr := fnhttp.NewRoundTripper( + tr := fnhttp.NewRoundTripper(nil, fnhttp.WithSelectCA(mockSelectCA), fnhttp.WithInClusterDialer(mockInCusterDialer)) defer tr.Close() diff --git a/pkg/k8s/configmaps.go b/pkg/k8s/configmaps.go index abf88e0767..a2d834171c 100644 --- a/pkg/k8s/configmaps.go +++ b/pkg/k8s/configmaps.go @@ -12,8 +12,8 @@ import ( k8sclientcmd "k8s.io/client-go/tools/clientcmd" ) -func GetConfigMap(ctx context.Context, name, namespaceOverride string) (*corev1.ConfigMap, error) { - client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) +func GetConfigMap(ctx context.Context, c *Client, name, namespaceOverride string) (*corev1.ConfigMap, error) { + client, namespace, err := c.ClientAndNamespace(namespaceOverride) if err != nil { return nil, err } @@ -23,8 +23,8 @@ func GetConfigMap(ctx context.Context, name, namespaceOverride string) (*corev1. // ListConfigMapsNamesIfConnected lists names of ConfigMaps present and the current k8s context // returns empty list, if not connected to any cluster -func ListConfigMapsNamesIfConnected(ctx context.Context, namespaceOverride string) (names []string, err error) { - names, err = listConfigMapsNames(ctx, namespaceOverride) +func ListConfigMapsNamesIfConnected(ctx context.Context, c *Client, namespaceOverride string) (names []string, err error) { + names, err = listConfigMapsNames(ctx, c, namespaceOverride) if err != nil { // not logged our authorized to access resources if k8serrors.IsForbidden(err) || k8serrors.IsUnauthorized(err) || k8serrors.IsInvalid(err) || k8serrors.IsTimeout(err) { @@ -53,8 +53,8 @@ func ListConfigMapsNamesIfConnected(ctx context.Context, namespaceOverride strin return } -func listConfigMapsNames(ctx context.Context, namespaceOverride string) (names []string, err error) { - client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) +func listConfigMapsNames(ctx context.Context, c *Client, namespaceOverride string) (names []string, err error) { + client, namespace, err := c.ClientAndNamespace(namespaceOverride) if err != nil { return } diff --git a/pkg/k8s/configmaps_test.go b/pkg/k8s/configmaps_test.go index 338163e9e0..26b315c822 100644 --- a/pkg/k8s/configmaps_test.go +++ b/pkg/k8s/configmaps_test.go @@ -8,7 +8,7 @@ import ( func TestListConfigMapsNamesIfConnectedWrongKubeconfig(t *testing.T) { t.Setenv("KUBECONFIG", "/tmp/non-existent.config") - _, err := k8s.ListConfigMapsNamesIfConnected(t.Context(), "") + _, err := k8s.ListConfigMapsNamesIfConnected(t.Context(), k8s.NewClientFromKubeconfig(), "") if err != nil { t.Fatal(err) } @@ -16,7 +16,7 @@ func TestListConfigMapsNamesIfConnectedWrongKubeconfig(t *testing.T) { func TestListConfigMapsNamesIfConnectedWrongKubernentesMaster(t *testing.T) { t.Setenv("KUBERNETES_MASTER", "/tmp/non-existent.config") - _, err := k8s.ListConfigMapsNamesIfConnected(t.Context(), "") + _, err := k8s.ListConfigMapsNamesIfConnected(t.Context(), k8s.NewClientFromKubeconfig(), "") if err != nil { t.Fatal(err) } diff --git a/pkg/k8s/deployer.go b/pkg/k8s/deployer.go index 203b8c255b..9d34a100bf 100644 --- a/pkg/k8s/deployer.go +++ b/pkg/k8s/deployer.go @@ -188,7 +188,7 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu return fn.DeploymentResult{}, fmt.Errorf("failed to generate deployment resources: %w", err) } - if err = CheckResourcesArePresent(ctx, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName, f.Deploy.ImagePullSecret); err != nil { + if err = CheckResourcesArePresent(ctx, d.kc, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName, f.Deploy.ImagePullSecret); err != nil { return fn.DeploymentResult{}, fmt.Errorf("failed to validate referenced resources: %w", err) } @@ -248,7 +248,7 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu return fn.DeploymentResult{}, fmt.Errorf("failed to generate deployment resources: %w", err) } - if err = CheckResourcesArePresent(ctx, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName, f.Deploy.ImagePullSecret); err != nil { + if err = CheckResourcesArePresent(ctx, d.kc, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName, f.Deploy.ImagePullSecret); err != nil { return fn.DeploymentResult{}, fmt.Errorf("failed to validate referenced resources: %w", err) } @@ -788,10 +788,10 @@ func withoutWorkloadAnnotations(annotations map[string]string) map[string]string // CheckResourcesArePresent returns error if Secrets or ConfigMaps // referenced in input sets are not deployed on the cluster in the specified namespace -func CheckResourcesArePresent(ctx context.Context, namespace string, referencedSecrets, referencedConfigMaps, referencedPVCs *sets.Set[string], referencedServiceAccount, imagePullSecret string) error { +func CheckResourcesArePresent(ctx context.Context, c *Client, namespace string, referencedSecrets, referencedConfigMaps, referencedPVCs *sets.Set[string], referencedServiceAccount, imagePullSecret string) error { errMsg := "" for s := range *referencedSecrets { - _, err := GetSecret(ctx, s, namespace) + _, err := GetSecret(ctx, c, s, namespace) if err != nil { if errors.IsForbidden(err) { errMsg += " Ensure that the service account has the necessary permissions to access the secret.\n" @@ -802,14 +802,14 @@ func CheckResourcesArePresent(ctx context.Context, namespace string, referencedS } for cm := range *referencedConfigMaps { - _, err := GetConfigMap(ctx, cm, namespace) + _, err := GetConfigMap(ctx, c, cm, namespace) if err != nil { errMsg += fmt.Sprintf(" referenced ConfigMap \"%s\" is not present in namespace \"%s\"\n", cm, namespace) } } for pvc := range *referencedPVCs { - _, err := GetPersistentVolumeClaim(ctx, pvc, namespace) + _, err := GetPersistentVolumeClaim(ctx, c, pvc, namespace) if err != nil { errMsg += fmt.Sprintf(" referenced PersistentVolumeClaim \"%s\" is not present in namespace \"%s\"\n", pvc, namespace) } @@ -817,14 +817,14 @@ func CheckResourcesArePresent(ctx context.Context, namespace string, referencedS // check if referenced ServiceAccount is present in the namespace if it is not default if referencedServiceAccount != "" && referencedServiceAccount != "default" { - err := GetServiceAccount(ctx, referencedServiceAccount, namespace) + err := GetServiceAccount(ctx, c, referencedServiceAccount, namespace) if err != nil { errMsg += fmt.Sprintf(" referenced ServiceAccount \"%s\" is not present in namespace \"%s\"\n", referencedServiceAccount, namespace) } } if imagePullSecret != "" { - _, err := GetSecret(ctx, imagePullSecret, namespace) + _, err := GetSecret(ctx, c, imagePullSecret, namespace) if err != nil { errMsg += fmt.Sprintf(" referenced image pull Secret \"%s\" is not present in namespace \"%s\"\n", imagePullSecret, namespace) } diff --git a/pkg/k8s/dialer.go b/pkg/k8s/dialer.go index 5fe559c794..af81b77440 100644 --- a/pkg/k8s/dialer.go +++ b/pkg/k8s/dialer.go @@ -24,7 +24,6 @@ import ( "k8s.io/client-go/kubernetes/scheme" v1 "k8s.io/client-go/kubernetes/typed/core/v1" restclient "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/remotecommand" ) @@ -48,10 +47,10 @@ var SocatImage = "ghcr.io/knative/func-utils:v2" // var client = http.Client{ // Transport: transport, // } -func NewInClusterDialer(ctx context.Context, clientConfig clientcmd.ClientConfig) (*contextDialer, error) { +func NewInClusterDialer(ctx context.Context, client *Client) (*contextDialer, error) { c := &contextDialer{ - clientConfig: clientConfig, - detachChan: make(chan struct{}), + client: client, + detachChan: make(chan struct{}), } err := c.startDialerPod(ctx) if err != nil { @@ -61,12 +60,12 @@ func NewInClusterDialer(ctx context.Context, clientConfig clientcmd.ClientConfig } type contextDialer struct { - coreV1 v1.CoreV1Interface - clientConfig clientcmd.ClientConfig - restConf *restclient.Config - podName string - namespace string - detachChan chan struct{} + coreV1 v1.CoreV1Interface + client *Client + restConf *restclient.Config + podName string + namespace string + detachChan chan struct{} } func (c *contextDialer) DialContext(ctx context.Context, network string, addr string) (net.Conn, error) { @@ -252,10 +251,11 @@ func (c *contextDialer) Close() error { } func (c *contextDialer) startDialerPod(ctx context.Context) (err error) { - c.restConf, err = c.clientConfig.ClientConfig() + c.restConf, err = c.client.RestConfig() if err != nil { return } + c.restConf = restclient.CopyConfig(c.restConf) // the Client's config is shared; do not mutate it c.restConf.WarningHandler = restclient.NoWarnings{} err = setConfigDefaults(c.restConf) @@ -269,7 +269,7 @@ func (c *contextDialer) startDialerPod(ctx context.Context) (err error) { } c.coreV1 = client.CoreV1() - c.namespace, _, err = c.clientConfig.Namespace() + c.namespace, err = c.client.DefaultNamespace() if err != nil { return } @@ -284,6 +284,7 @@ func (c *contextDialer) startDialerPod(ctx context.Context) (err error) { } }() + openShift, _ := c.client.IsOpenShift() pod := &coreV1.Pod{ ObjectMeta: metaV1.ObjectMeta{ Name: c.podName, @@ -291,7 +292,7 @@ func (c *contextDialer) startDialerPod(ctx context.Context) (err error) { Annotations: nil, }, Spec: coreV1.PodSpec{ - SecurityContext: defaultPodSecurityContext(), + SecurityContext: defaultPodSecurityContext(openShift), Containers: []coreV1.Container{ { Name: c.podName, @@ -433,7 +434,7 @@ func podReady(ctx context.Context, core v1.CoreV1Interface, podName, namespace s return } if status.State.Terminated != nil { - msg, _ := GetPodLogs(ctx, namespace, podName, podName) + msg, _ := podLogs(ctx, core, namespace, podName, podName) d <- fmt.Errorf("pod prematurely exited (output: %q, exitcode: %d)", msg, status.State.Terminated.ExitCode) return } @@ -555,14 +556,14 @@ func newConn() (*io.PipeReader, *io.PipeWriter, *conn) { return pr1, pw0, rwc } -func NewLazyInitInClusterDialer(clientConfig clientcmd.ClientConfig) *lazyInitInClusterDialer { +func NewLazyInitInClusterDialer(client *Client) *lazyInitInClusterDialer { return &lazyInitInClusterDialer{ - clientConfig: clientConfig, + client: client, } } type lazyInitInClusterDialer struct { - clientConfig clientcmd.ClientConfig + client *Client contextDialer *contextDialer initErr error o sync.Once @@ -570,7 +571,7 @@ type lazyInitInClusterDialer struct { func (l *lazyInitInClusterDialer) DialContext(ctx context.Context, network string, addr string) (net.Conn, error) { l.o.Do(func() { - l.contextDialer, l.initErr = NewInClusterDialer(ctx, l.clientConfig) + l.contextDialer, l.initErr = NewInClusterDialer(ctx, l.client) }) if l.initErr != nil { return nil, l.initErr diff --git a/pkg/k8s/dialer_int_test.go b/pkg/k8s/dialer_int_test.go index b2b40dd36c..c1a737865c 100644 --- a/pkg/k8s/dialer_int_test.go +++ b/pkg/k8s/dialer_int_test.go @@ -157,7 +157,7 @@ func TestInt_DialInClusterService(t *testing.T) { // Initialize the InClusterDialer. This will create a socat pod in the // cluster that acts as a TCP proxy, allowing us to reach cluster-internal // services. The "lazy init" variant only creates the pod when first used. - dialer := k8s.NewLazyInitInClusterDialer(clientConfig) + dialer := k8s.NewLazyInitInClusterDialer(k8s.NewClient(clientConfig)) t.Cleanup(func() { dialer.Close() }) @@ -222,7 +222,7 @@ func TestInt_DialInClusterService(t *testing.T) { func TestInt_DialUnreachable(t *testing.T) { var ctx = t.Context() - dialer, err := k8s.NewInClusterDialer(ctx, k8s.GetClientConfig()) + dialer, err := k8s.NewInClusterDialer(ctx, k8s.NewClientFromKubeconfig()) if err != nil { t.Fatal(err) } @@ -326,7 +326,7 @@ func TestInt_DialContextExpiry(t *testing.T) { } // Create the dialer pod eagerly so that pod creation is not tied to dialCtx. - dialer, err := k8s.NewInClusterDialer(setupCtx, clientConfig) + dialer, err := k8s.NewInClusterDialer(setupCtx, k8s.NewClient(clientConfig)) if err != nil { t.Fatal(err) } diff --git a/pkg/k8s/logs.go b/pkg/k8s/logs.go index db7f94588e..599e132532 100644 --- a/pkg/k8s/logs.go +++ b/pkg/k8s/logs.go @@ -15,18 +15,28 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes" + v1 "k8s.io/client-go/kubernetes/typed/core/v1" ) // GetPodLogs returns logs from a specified Container in a Pod, if container is empty string, // then the first container in the pod is selected. -func GetPodLogs(ctx context.Context, namespace, podName, containerName string) (string, error) { +func GetPodLogs(ctx context.Context, c *Client, namespace, podName, containerName string) (string, error) { + client, namespace, err := c.ClientAndNamespace(namespace) + if err != nil { + return "", err + } + return podLogs(ctx, client.CoreV1(), namespace, podName, containerName) +} + +// podLogs returns logs of one container of a pod; an empty containerName +// selects the first container. +func podLogs(ctx context.Context, core v1.CoreV1Interface, namespace, podName, containerName string) (string, error) { podLogOpts := corev1.PodLogOptions{} if containerName != "" { podLogOpts.Container = containerName } - client, namespace, _ := NewClientAndResolvedNamespace(namespace) - request := client.CoreV1().Pods(namespace).GetLogs(podName, &podLogOpts) + request := core.Pods(namespace).GetLogs(podName, &podLogOpts) containerLogStream, err := request.Stream(ctx) if err != nil { @@ -96,8 +106,8 @@ type PodLogsOptions struct { // When Follow is set, this function runs as long as the passed context is active // (i.e. it is required to cancel the context to stop log gathering). Otherwise a // snapshot of the currently available logs is written and the function returns. -func GetPodLogsBySelector(ctx context.Context, opts PodLogsOptions, out io.Writer) error { - client, namespace, err := NewClientAndResolvedNamespace(opts.Namespace) +func GetPodLogsBySelector(ctx context.Context, c *Client, opts PodLogsOptions, out io.Writer) error { + client, namespace, err := c.ClientAndNamespace(opts.Namespace) if err != nil { return fmt.Errorf("cannot create k8s client: %w", err) } diff --git a/pkg/k8s/logs_int_test.go b/pkg/k8s/logs_int_test.go index 7ef95e4560..9bc94f3133 100644 --- a/pkg/k8s/logs_int_test.go +++ b/pkg/k8s/logs_int_test.go @@ -82,7 +82,7 @@ out: time.Sleep(time.Millisecond * 500) } - out, err := k8s.GetPodLogs(ctx, testingNS, testingPodName, testingPodName) + out, err := k8s.GetPodLogs(ctx, k8s.NewClientFromKubeconfig(), testingNS, testingPodName, testingPodName) if err != nil { t.Fatal(err) } @@ -126,7 +126,7 @@ func TestInt_GetPodLogsBySelectorSnapshot(t *testing.T) { // A snapshot of a selector which matches no pods is not an error, but is // reported such that it is distinguishable from a pod logging nothing. - err = k8s.GetPodLogsBySelector(ctx, opts, io.Discard) + err = k8s.GetPodLogsBySelector(ctx, k8s.NewClientFromKubeconfig(), opts, io.Discard) if !errors.Is(err, k8s.ErrNoMatchingPods) { t.Errorf("expected ErrNoMatchingPods for a selector matching no pods, got %v", err) } @@ -171,7 +171,7 @@ out: buff := &k8s.SynchronizedBuffer{} done := make(chan error, 1) go func() { - done <- k8s.GetPodLogsBySelector(ctx, opts, buff) + done <- k8s.GetPodLogsBySelector(ctx, k8s.NewClientFromKubeconfig(), opts, buff) }() select { diff --git a/pkg/k8s/manifestival.go b/pkg/k8s/manifestival.go index 7909d4f84a..d8fd0b8936 100644 --- a/pkg/k8s/manifestival.go +++ b/pkg/k8s/manifestival.go @@ -5,8 +5,8 @@ import ( "github.com/manifestival/manifestival" ) -func GetManifestivalClient() (manifestival.Client, error) { - config, err := GetClientConfig().ClientConfig() +func GetManifestivalClient(c *Client) (manifestival.Client, error) { + config, err := c.RestConfig() if err != nil { return nil, err } diff --git a/pkg/k8s/persistent_volumes.go b/pkg/k8s/persistent_volumes.go index 266af6a260..248159929a 100644 --- a/pkg/k8s/persistent_volumes.go +++ b/pkg/k8s/persistent_volumes.go @@ -23,8 +23,8 @@ import ( k8sclientcmd "k8s.io/client-go/tools/clientcmd" ) -func GetPersistentVolumeClaim(ctx context.Context, name, namespaceOverride string) (*corev1.PersistentVolumeClaim, error) { - client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) +func GetPersistentVolumeClaim(ctx context.Context, c *Client, name, namespaceOverride string) (*corev1.PersistentVolumeClaim, error) { + client, namespace, err := c.ClientAndNamespace(namespaceOverride) if err != nil { return nil, err } @@ -32,8 +32,8 @@ func GetPersistentVolumeClaim(ctx context.Context, name, namespaceOverride strin return client.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, name, metav1.GetOptions{}) } -func CreatePersistentVolumeClaim(ctx context.Context, name, namespaceOverride string, labels map[string]string, annotations map[string]string, accessMode corev1.PersistentVolumeAccessMode, resourceRequest resource.Quantity, storageClassName string) (err error) { - client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) +func CreatePersistentVolumeClaim(ctx context.Context, c *Client, name, namespaceOverride string, labels map[string]string, annotations map[string]string, accessMode corev1.PersistentVolumeAccessMode, resourceRequest resource.Quantity, storageClassName string) (err error) { + client, namespace, err := c.ClientAndNamespace(namespaceOverride) if err != nil { return } @@ -63,8 +63,8 @@ func CreatePersistentVolumeClaim(ctx context.Context, name, namespaceOverride st return } -func DeletePersistentVolumeClaims(ctx context.Context, namespaceOverride string, listOptions metav1.ListOptions) (err error) { - client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) +func DeletePersistentVolumeClaims(ctx context.Context, c *Client, namespaceOverride string, listOptions metav1.ListOptions) (err error) { + client, namespace, err := c.ClientAndNamespace(namespaceOverride) if err != nil { return } @@ -75,20 +75,20 @@ func DeletePersistentVolumeClaims(ctx context.Context, namespaceOverride string, var TarImage = "ghcr.io/knative/func-utils:v2" // UploadToVolume uploads files (passed in form of tar stream) into volume. -func UploadToVolume(ctx context.Context, content io.Reader, claimName, namespace string) error { - return runWithVolumeMounted(ctx, TarImage, []string{"sh", "-c", "umask 0000 && exec tar -xmf -"}, content, claimName, namespace) +func UploadToVolume(ctx context.Context, c *Client, content io.Reader, claimName, namespace string) error { + return runWithVolumeMounted(ctx, c, TarImage, []string{"sh", "-c", "umask 0000 && exec tar -xmf -"}, content, claimName, namespace) } // Runs a pod with given image, command and stdin // while having the volume mounted and working directory set to it. -func runWithVolumeMounted(ctx context.Context, podImage string, podCommand []string, podInput io.Reader, claimName, namespace string) error { +func runWithVolumeMounted(ctx context.Context, c *Client, podImage string, podCommand []string, podInput io.Reader, claimName, namespace string) error { var err error - cliConf := GetClientConfig() - restConf, err := cliConf.ClientConfig() + restConf, err := c.RestConfig() if err != nil { return fmt.Errorf("cannot get client config: %w", err) } + restConf = restclient.CopyConfig(restConf) // the Client's config is shared; do not mutate it restConf.WarningHandler = restclient.NoWarnings{} err = setConfigDefaults(restConf) @@ -102,7 +102,7 @@ func runWithVolumeMounted(ctx context.Context, podImage string, podCommand []str } if namespace == "" { - namespace, err = GetDefaultNamespace() + namespace, err = c.DefaultNamespace() if err != nil { return fmt.Errorf("cannot get namespace: %w", err) } @@ -118,6 +118,7 @@ func runWithVolumeMounted(ctx context.Context, podImage string, podCommand []str const volumeMntPoint = "/tmp/volume_mnt" const pVol = "p-vol" + openShift, _ := c.IsOpenShift() pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: podName, @@ -125,7 +126,7 @@ func runWithVolumeMounted(ctx context.Context, podImage string, podCommand []str Annotations: nil, }, Spec: corev1.PodSpec{ - SecurityContext: defaultPodSecurityContext(), + SecurityContext: defaultPodSecurityContext(openShift), Containers: []corev1.Container{ { Name: podName, @@ -269,8 +270,8 @@ func (t *tsBuff) Write(p []byte) (n int, err error) { // ListPersistentVolumeClaimsNamesIfConnected lists names of PersistentVolumeClaims present and the current k8s context // returns empty list, if not connected to any cluster -func ListPersistentVolumeClaimsNamesIfConnected(ctx context.Context, namespaceOverride string) (names []string, err error) { - names, err = listPersistentVolumeClaimsNames(ctx, namespaceOverride) +func ListPersistentVolumeClaimsNamesIfConnected(ctx context.Context, c *Client, namespaceOverride string) (names []string, err error) { + names, err = listPersistentVolumeClaimsNames(ctx, c, namespaceOverride) if err != nil { // not logged our authorized to access resources if k8serrors.IsForbidden(err) || k8serrors.IsUnauthorized(err) || k8serrors.IsInvalid(err) || k8serrors.IsTimeout(err) { @@ -299,8 +300,8 @@ func ListPersistentVolumeClaimsNamesIfConnected(ctx context.Context, namespaceOv return } -func listPersistentVolumeClaimsNames(ctx context.Context, namespaceOverride string) (names []string, err error) { - client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) +func listPersistentVolumeClaimsNames(ctx context.Context, c *Client, namespaceOverride string) (names []string, err error) { + client, namespace, err := c.ClientAndNamespace(namespaceOverride) if err != nil { return } diff --git a/pkg/k8s/persistent_volumes_int_test.go b/pkg/k8s/persistent_volumes_int_test.go index 2897a4946d..511e833efd 100644 --- a/pkg/k8s/persistent_volumes_int_test.go +++ b/pkg/k8s/persistent_volumes_int_test.go @@ -24,7 +24,8 @@ func TestInt_UploadToVolume(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), time.Minute*5) t.Cleanup(cancel) - cliSet, testingNS, err := k8s.NewClientAndResolvedNamespace("") + kc := k8s.NewClientFromKubeconfig() + cliSet, testingNS, err := kc.ClientAndNamespace("") if err != nil { t.Fatal(err) } @@ -32,7 +33,7 @@ func TestInt_UploadToVolume(t *testing.T) { rnd := rand.String(5) testingPVCName := "testing-pvc-" + rnd - err = k8s.CreatePersistentVolumeClaim(ctx, testingPVCName, testingNS, + err = k8s.CreatePersistentVolumeClaim(ctx, kc, testingPVCName, testingNS, nil, nil, corev1.ReadWriteOnce, *resource.NewQuantity(1024, resource.DecimalSI), "") if err != nil { @@ -48,7 +49,7 @@ func TestInt_UploadToVolume(t *testing.T) { t.Log("created PVC: " + testingPVCName) // First, test error handling by uploading empty content stream. - err = k8s.UploadToVolume(ctx, &bytes.Buffer{}, testingPVCName, testingNS) + err = k8s.UploadToVolume(ctx, kc, &bytes.Buffer{}, testingPVCName, testingNS) if err == nil || !strings.Contains(err.Error(), "does not look like a tar") { t.Error("got error, or error with unexpected message") } @@ -59,7 +60,7 @@ func TestInt_UploadToVolume(t *testing.T) { } t.Cleanup(func() { f.Close() }) - err = k8s.UploadToVolume(ctx, f, testingPVCName, testingNS) + err = k8s.UploadToVolume(ctx, kc, f, testingPVCName, testingNS) if err != nil { t.Fatal(err) } @@ -126,7 +127,7 @@ func TestInt_UploadToVolume(t *testing.T) { } t.Log("the testing pod has exited") - out, err := k8s.GetPodLogs(ctx, testingNS, testingPodName, testingPodName) + out, err := k8s.GetPodLogs(ctx, k8s.NewClientFromKubeconfig(), testingNS, testingPodName, testingPodName) if err != nil { t.Fatal(err) } @@ -138,7 +139,7 @@ func TestInt_UploadToVolume(t *testing.T) { func TestInt_ListPersistentVolumeClaimsNamesIfConnectedWrongKubeconfig(t *testing.T) { t.Setenv("KUBECONFIG", "/tmp/non-existent.config") - _, err := k8s.ListPersistentVolumeClaimsNamesIfConnected(t.Context(), "") + _, err := k8s.ListPersistentVolumeClaimsNamesIfConnected(t.Context(), k8s.NewClientFromKubeconfig(), "") if err != nil { t.Fatal(err) } @@ -146,7 +147,7 @@ func TestInt_ListPersistentVolumeClaimsNamesIfConnectedWrongKubeconfig(t *testin func TestInt_ListPersistentVolumeClaimsNamesIfConnectedWrongKubernentesMaster(t *testing.T) { t.Setenv("KUBERNETES_MASTER", "/tmp/non-existent.config") - _, err := k8s.ListPersistentVolumeClaimsNamesIfConnected(t.Context(), "") + _, err := k8s.ListPersistentVolumeClaimsNamesIfConnected(t.Context(), k8s.NewClientFromKubeconfig(), "") if err != nil { t.Fatal(err) } diff --git a/pkg/k8s/secrets.go b/pkg/k8s/secrets.go index f8380a3414..7907b5bfdd 100644 --- a/pkg/k8s/secrets.go +++ b/pkg/k8s/secrets.go @@ -15,8 +15,8 @@ import ( k8sclientcmd "k8s.io/client-go/tools/clientcmd" ) -func GetSecret(ctx context.Context, name, namespaceOverride string) (*corev1.Secret, error) { - client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) +func GetSecret(ctx context.Context, c *Client, name, namespaceOverride string) (*corev1.Secret, error) { + client, namespace, err := c.ClientAndNamespace(namespaceOverride) if err != nil { return nil, err } @@ -26,8 +26,8 @@ func GetSecret(ctx context.Context, name, namespaceOverride string) (*corev1.Sec // ListSecretsNamesIfConnected lists names of Secrets present and the current k8s context // returns empty list, if not connected to any cluster -func ListSecretsNamesIfConnected(ctx context.Context, namespaceOverride string) (names []string, err error) { - names, err = listSecretsNames(ctx, namespaceOverride) +func ListSecretsNamesIfConnected(ctx context.Context, c *Client, namespaceOverride string) (names []string, err error) { + names, err = listSecretsNames(ctx, c, namespaceOverride) if err != nil { // not logged our authorized to access resources if k8serrors.IsForbidden(err) || k8serrors.IsUnauthorized(err) || k8serrors.IsInvalid(err) || k8serrors.IsTimeout(err) { @@ -56,8 +56,8 @@ func ListSecretsNamesIfConnected(ctx context.Context, namespaceOverride string) return } -func listSecretsNames(ctx context.Context, namespaceOverride string) (names []string, err error) { - client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) +func listSecretsNames(ctx context.Context, c *Client, namespaceOverride string) (names []string, err error) { + client, namespace, err := c.ClientAndNamespace(namespaceOverride) if err != nil { return } @@ -74,8 +74,8 @@ func listSecretsNames(ctx context.Context, namespaceOverride string) (names []st return } -func DeleteSecrets(ctx context.Context, namespaceOverride string, listOptions metav1.ListOptions) (err error) { - client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) +func DeleteSecrets(ctx context.Context, c *Client, namespaceOverride string, listOptions metav1.ListOptions) (err error) { + client, namespace, err := c.ClientAndNamespace(namespaceOverride) if err != nil { return } @@ -83,7 +83,7 @@ func DeleteSecrets(ctx context.Context, namespaceOverride string, listOptions me return client.CoreV1().Secrets(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, listOptions) } -func EnsureDockerRegistrySecretExist(ctx context.Context, name, namespaceOverride string, labels map[string]string, annotations map[string]string, username, password, server string) (err error) { +func EnsureDockerRegistrySecretExist(ctx context.Context, c *Client, name, namespaceOverride string, labels map[string]string, annotations map[string]string, username, password, server string) (err error) { dockerConfigJSONContent, err := HandleDockerCfgJSONContent(username, password, "", server) if err != nil { return @@ -101,18 +101,18 @@ func EnsureDockerRegistrySecretExist(ctx context.Context, name, namespaceOverrid } secret.Data["config.json"] = dockerConfigJSONContent - return EnsureSecretExist(ctx, secret, namespaceOverride) + return EnsureSecretExist(ctx, c, secret, namespaceOverride) } -func EnsureSecretExist(ctx context.Context, secret corev1.Secret, namespaceOverride string) (err error) { - client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) +func EnsureSecretExist(ctx context.Context, c *Client, secret corev1.Secret, namespaceOverride string) (err error) { + client, namespace, err := c.ClientAndNamespace(namespaceOverride) if err != nil { return } // Check whether Secret with specified name exist secretNotFound := false - existingSecret, err := GetSecret(ctx, secret.Name, namespace) + existingSecret, err := GetSecret(ctx, c, secret.Name, namespace) if err != nil { if !k8serrors.IsNotFound(err) { return diff --git a/pkg/k8s/secrets_test.go b/pkg/k8s/secrets_test.go index eafcc795f1..f8b330e46b 100644 --- a/pkg/k8s/secrets_test.go +++ b/pkg/k8s/secrets_test.go @@ -8,7 +8,7 @@ import ( func TestListSecretsNamesIfConnectedWrongKubeconfig(t *testing.T) { t.Setenv("KUBECONFIG", "/tmp/non-existent.config") - _, err := k8s.ListSecretsNamesIfConnected(t.Context(), "") + _, err := k8s.ListSecretsNamesIfConnected(t.Context(), k8s.NewClientFromKubeconfig(), "") if err != nil { t.Fatal(err) } @@ -16,7 +16,7 @@ func TestListSecretsNamesIfConnectedWrongKubeconfig(t *testing.T) { func TestListSecretsNamesIfConnectedWrongKubernentesMaster(t *testing.T) { t.Setenv("KUBERNETES_MASTER", "/tmp/non-existent.config") - _, err := k8s.ListSecretsNamesIfConnected(t.Context(), "") + _, err := k8s.ListSecretsNamesIfConnected(t.Context(), k8s.NewClientFromKubeconfig(), "") if err != nil { t.Fatal(err) } diff --git a/pkg/k8s/security_context.go b/pkg/k8s/security_context.go index 4ddaecbf01..51c7837d09 100644 --- a/pkg/k8s/security_context.go +++ b/pkg/k8s/security_context.go @@ -16,11 +16,11 @@ import ( // with Tekton buildpack tasks that mount volumes with group ownership 0. // This does not violate the restricted profile (which checks UID, not GID) but is // tracked for remediation in https://github.com/knative/func/issues/3517. -func defaultPodSecurityContext() *corev1.PodSecurityContext { +func defaultPodSecurityContext(openShift bool) *corev1.PodSecurityContext { runAsNonRoot := true seccompProfile := &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault} - if IsOpenShift() { + if openShift { // On OpenShift, SCCs manage RunAsUser/RunAsGroup/FSGroup; setting them // here would conflict with the namespace's SCC UID range policy. // Only set the fields required by the restricted PSA profile. diff --git a/pkg/k8s/security_context_test.go b/pkg/k8s/security_context_test.go index 9c096c6ea3..30832c8210 100644 --- a/pkg/k8s/security_context_test.go +++ b/pkg/k8s/security_context_test.go @@ -6,15 +6,8 @@ import ( corev1 "k8s.io/api/core/v1" ) -// Note: SetOpenShiftForTest mutates a package-level bool without a mutex. -// These tests must not be run with t.Parallel() until that is addressed. -// See openshift.go:SetOpenShiftForTest. - func TestDefaultPodSecurityContext_NonOpenShift(t *testing.T) { - cleanup := SetOpenShiftForTest(false, nil) - defer cleanup() - - sc := defaultPodSecurityContext() + sc := defaultPodSecurityContext(false) if sc == nil { t.Fatal("expected non-nil PodSecurityContext on non-OpenShift") } @@ -36,10 +29,7 @@ func TestDefaultPodSecurityContext_NonOpenShift(t *testing.T) { } func TestDefaultPodSecurityContext_OpenShift(t *testing.T) { - cleanup := SetOpenShiftForTest(true, nil) - defer cleanup() - - sc := defaultPodSecurityContext() + sc := defaultPodSecurityContext(true) if sc == nil { t.Fatal("expected non-nil PodSecurityContext on OpenShift") } @@ -107,10 +97,7 @@ func TestRestrictedProfileCompliance(t *testing.T) { name = "openshift" } t.Run(name, func(t *testing.T) { - cleanup := SetOpenShiftForTest(openshift, nil) - defer cleanup() - - pod := defaultPodSecurityContext() + pod := defaultPodSecurityContext(openshift) ctr := defaultSecurityContext() // restricted requires: allowPrivilegeEscalation=false (container level) diff --git a/pkg/k8s/serviceaccount.go b/pkg/k8s/serviceaccount.go index 8180cae270..47f6fa2f0c 100644 --- a/pkg/k8s/serviceaccount.go +++ b/pkg/k8s/serviceaccount.go @@ -6,8 +6,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func GetServiceAccount(ctx context.Context, referencedServiceAccount, namespace string) error { - k8sClient, err := NewKubernetesClientset() +func GetServiceAccount(ctx context.Context, c *Client, referencedServiceAccount, namespace string) error { + k8sClient, err := c.Clientset() if err != nil { return err } diff --git a/pkg/knative/deployer.go b/pkg/knative/deployer.go index 4aa7e46401..3992407521 100644 --- a/pkg/knative/deployer.go +++ b/pkg/knative/deployer.go @@ -176,10 +176,10 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu daprInstalled = true } - t := fnhttp.NewRoundTripper( + t := fnhttp.NewRoundTripper(kc, fnhttp.WithOpenShiftServiceCA(kc), fnhttp.WithInsecureSkipVerify(f.RegistryInsecure), - fnhttp.WithInClusterDialer(k8s.NewLazyInitInClusterDialer(kc.Loader())), + fnhttp.WithInClusterDialer(k8s.NewLazyInitInClusterDialer(kc)), ) defer func(t fnhttp.RoundTripCloser) { _ = t.Close() @@ -208,7 +208,7 @@ consider using the --image-pull-secret flag, or setting up pull secrets manually } since := time.Now() go func() { - _ = GetKServiceLogs(ctx, LogsOptions{ + _ = GetKServiceLogs(ctx, kc, LogsOptions{ Name: f.Name, Namespace: namespace, Image: f.Deploy.Image, @@ -234,7 +234,7 @@ consider using the --image-pull-secret flag, or setting up pull secrets manually return fn.DeploymentResult{}, err } - err = k8s.CheckResourcesArePresent(ctx, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName, f.Deploy.ImagePullSecret) + err = k8s.CheckResourcesArePresent(ctx, kc, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName, f.Deploy.ImagePullSecret) if err != nil { err = fmt.Errorf("knative deployer failed to generate the Knative Service: %v", err) return fn.DeploymentResult{}, err @@ -338,7 +338,7 @@ consider using the --image-pull-secret flag, or setting up pull secrets manually return fn.DeploymentResult{}, err } - err = k8s.CheckResourcesArePresent(ctx, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName, f.Deploy.ImagePullSecret) + err = k8s.CheckResourcesArePresent(ctx, kc, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName, f.Deploy.ImagePullSecret) if err != nil { err = fmt.Errorf("knative deployer failed to update the Knative Service: %v", err) return fn.DeploymentResult{}, err diff --git a/pkg/knative/logs.go b/pkg/knative/logs.go index b6613a5371..12afff3632 100644 --- a/pkg/knative/logs.go +++ b/pkg/knative/logs.go @@ -43,8 +43,8 @@ type LogsOptions struct { // Otherwise a snapshot of the currently available logs is written and the // function returns, with k8s.ErrNoMatchingPods if the service currently has no // pods whose logs can be read. -func GetKServiceLogs(ctx context.Context, opts LogsOptions, out io.Writer) error { - return k8s.GetPodLogsBySelector(ctx, k8s.PodLogsOptions{ +func GetKServiceLogs(ctx context.Context, kc *k8s.Client, opts LogsOptions, out io.Writer) error { + return k8s.GetPodLogsBySelector(ctx, kc, k8s.PodLogsOptions{ Namespace: opts.Namespace, LabelSelector: fmt.Sprintf("serving.knative.dev/service=%s", opts.Name), Container: "user-container", diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index c067abe005..7031cd1d05 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -44,7 +44,10 @@ var ensureRegistrySecret = k8s.EnsureDockerRegistrySecretExist // It sets up Kubernetes clients, checks if the Function CRD exists on the // cluster, and creates or updates the CR accordingly. func SyncFunctionCR(ctx context.Context, cfg SyncConfig) error { - restCfg, err := k8s.GetClientConfig().ClientConfig() + // TODO: inject the k8s.Client through NewSyncer instead of resolving it + // from the kubeconfig here. + kc := k8s.NewClientFromKubeconfig() + restCfg, err := kc.RestConfig() if err != nil { return fmt.Errorf("getting kubernetes config: %w", err) } @@ -64,10 +67,10 @@ func SyncFunctionCR(ctx context.Context, cfg SyncConfig) error { return fmt.Errorf("creating kubernetes client: %w", err) } - return syncFunctionCR(ctx, cl, disc, cfg) + return syncFunctionCR(ctx, kc, cl, disc, cfg) } -func syncFunctionCR(ctx context.Context, cl ctrlclient.Client, disc discovery.DiscoveryInterface, cfg SyncConfig) error { +func syncFunctionCR(ctx context.Context, kc *k8s.Client, cl ctrlclient.Client, disc discovery.DiscoveryInterface, cfg SyncConfig) error { hasCRD, err := hasFunctionCRD(disc) if err != nil { return fmt.Errorf("checking for Function CRD: %w", err) @@ -84,7 +87,7 @@ func syncFunctionCR(ctx context.Context, cl ctrlclient.Client, disc discovery.Di var registrySecretRef *v1.LocalObjectReference if cfg.RegistryCredentials != nil { secretName := cfg.FunctionName + "-registry-auth" - if err := ensureRegistrySecret(ctx, secretName, cfg.Namespace, nil, nil, + if err := ensureRegistrySecret(ctx, kc, secretName, cfg.Namespace, nil, nil, cfg.RegistryCredentials.Username, cfg.RegistryCredentials.Password, cfg.RegistryCredentials.Server); err != nil { return fmt.Errorf("creating registry secret: %w", err) } diff --git a/pkg/operator/sync_test.go b/pkg/operator/sync_test.go index e59fe8cb9c..f21f636a13 100644 --- a/pkg/operator/sync_test.go +++ b/pkg/operator/sync_test.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" v1alpha1 "github.com/functions-dev/func-operator/api/v1alpha1" + "knative.dev/func/pkg/k8s" ) func newScheme() *runtime.Scheme { @@ -53,7 +54,7 @@ func TestSyncFunctionCR_CreateNew(t *testing.T) { RepoPath: ".", } - err := syncFunctionCR(context.Background(), cl, disc, cfg) + err := syncFunctionCR(context.Background(), nil, cl, disc, cfg) if err != nil { t.Fatal(err) } @@ -102,7 +103,7 @@ func TestSyncFunctionCR_UpdateExistingByMetadataName(t *testing.T) { RepoPath: "subfolder", } - err := syncFunctionCR(context.Background(), cl, disc, cfg) + err := syncFunctionCR(context.Background(), nil, cl, disc, cfg) if err != nil { t.Fatal(err) } @@ -161,7 +162,7 @@ func TestSyncFunctionCR_UpdateExistingByStatusName(t *testing.T) { RepoPath: ".", } - err := syncFunctionCR(context.Background(), cl, disc, cfg) + err := syncFunctionCR(context.Background(), nil, cl, disc, cfg) if err != nil { t.Fatal(err) } @@ -196,7 +197,7 @@ func TestSyncFunctionCR_NoCRD_SkipSilently(t *testing.T) { RepoPath: ".", } - err := syncFunctionCR(context.Background(), cl, disc, cfg) + err := syncFunctionCR(context.Background(), nil, cl, disc, cfg) if err != nil { t.Fatalf("expected no error when CRD missing, got: %v", err) } @@ -204,7 +205,7 @@ func TestSyncFunctionCR_NoCRD_SkipSilently(t *testing.T) { func TestSyncFunctionCR_WithRegistryCredentials(t *testing.T) { original := ensureRegistrySecret - ensureRegistrySecret = func(_ context.Context, _, _ string, _, _ map[string]string, _, _, _ string) error { + ensureRegistrySecret = func(_ context.Context, _ *k8s.Client, _, _ string, _, _ map[string]string, _, _, _ string) error { return nil } t.Cleanup(func() { ensureRegistrySecret = original }) @@ -226,7 +227,7 @@ func TestSyncFunctionCR_WithRegistryCredentials(t *testing.T) { }, } - err := syncFunctionCR(context.Background(), cl, disc, cfg) + err := syncFunctionCR(context.Background(), nil, cl, disc, cfg) if err != nil { t.Fatal(err) } @@ -260,7 +261,7 @@ func TestSyncFunctionCR_NoRepoURL_SkipsWithMessage(t *testing.T) { RepoPath: ".", } - err := syncFunctionCR(context.Background(), cl, disc, cfg) + err := syncFunctionCR(context.Background(), nil, cl, disc, cfg) if err != nil { t.Fatalf("expected no error when repo URL empty, got: %v", err) } diff --git a/pkg/pipelines/tekton/pipelines_int_test.go b/pkg/pipelines/tekton/pipelines_int_test.go index ba13781302..1a0a60da01 100644 --- a/pkg/pipelines/tekton/pipelines_int_test.go +++ b/pkg/pipelines/tekton/pipelines_int_test.go @@ -97,7 +97,7 @@ func assertFunctionEchoes(httpClient *http.Client, url string) (err error) { func httpClientForDeployer(t *testing.T, ctx context.Context, deployer string) *http.Client { switch deployer { case k8s.KubernetesDeployerName, keda.KedaDeployerName: - dialer, err := k8s.NewInClusterDialer(ctx, k8s.GetClientConfig()) + dialer, err := k8s.NewInClusterDialer(ctx, k8s.NewClientFromKubeconfig()) if err != nil { t.Fatalf("failed to create in-cluster dialer: %v", err) } diff --git a/pkg/pipelines/tekton/pipelines_pac_provider.go b/pkg/pipelines/tekton/pipelines_pac_provider.go index 87e3bd2fd6..d04bfc527d 100644 --- a/pkg/pipelines/tekton/pipelines_pac_provider.go +++ b/pkg/pipelines/tekton/pipelines_pac_provider.go @@ -63,7 +63,7 @@ func (pp *PipelinesProvider) ConfigurePAC(ctx context.Context, f fn.Function, me data.WebhookSecret = random.AlphaString(10) // try to reuse existing Webhook Secret stored in the cluster - secret, err := k8s.GetSecret(ctx, getPipelineSecretName(f), namespace) + secret, err := k8s.GetSecret(ctx, pp.kc, getPipelineSecretName(f), namespace) if err != nil { if !k8serrors.IsNotFound(err) { return err @@ -134,12 +134,12 @@ func (pp *PipelinesProvider) createLocalPACResources(ctx context.Context, f fn.F labels = pp.decorator.UpdateLabels(f, labels) } - err = createPipelineTemplatePAC(f, labels) + err = createPipelineTemplatePAC(pp.kc, f, labels) if err != nil { return err } - err = createPipelineRunTemplatePAC(f, labels) + err = createPipelineRunTemplatePAC(pp.kc, f, labels) if err != nil { return err } @@ -201,13 +201,13 @@ func (pp *PipelinesProvider) createClusterPACResources(ctx context.Context, f fn metadata.RegistryPassword = creds.Password metadata.RegistryServer = registry - err = createPipelinePersistentVolumeClaim(ctx, f, namespace, labels) + err = createPipelinePersistentVolumeClaim(ctx, pp.kc, f, namespace, labels) if err != nil { return err } fmt.Printf(" ✅ Persistent Volume is present on the cluster with name %q\n", getPipelinePvcName(f)) - err = ensurePACSecretExists(ctx, f, namespace, metadata, labels) + err = ensurePACSecretExists(ctx, pp.kc, f, namespace, metadata, labels) if err != nil { return err } diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index f384c60112..f2f2dc8a58 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -188,7 +188,7 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn labels = pp.decorator.UpdateLabels(f, labels) } - err = createPipelinePersistentVolumeClaim(ctx, f, namespace, labels) + err = createPipelinePersistentVolumeClaim(ctx, pp.kc, f, namespace, labels) if err != nil { return "", f, err } @@ -197,13 +197,13 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn // Use direct upload to PVC if Git is not set up. content := sourcesAsTarStream(f) defer content.Close() - err = k8s.UploadToVolume(ctx, content, getPipelinePvcName(f), namespace) + err = k8s.UploadToVolume(ctx, pp.kc, content, getPipelinePvcName(f), namespace) if err != nil { return "", f, fmt.Errorf("cannot upload sources to the PVC: %w", err) } } - err = createAndApplyPipelineTemplate(f, namespace, labels) + err = createAndApplyPipelineTemplate(pp.kc, f, namespace, labels) if err != nil { if !k8serrors.IsAlreadyExists(err) { if k8serrors.IsNotFound(err) { @@ -235,12 +235,12 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn f.Registry = registry } - err = k8s.EnsureDockerRegistrySecretExist(ctx, getPipelineSecretName(f), namespace, labels, f.Deploy.Annotations, creds.Username, creds.Password, registry) + err = k8s.EnsureDockerRegistrySecretExist(ctx, pp.kc, getPipelineSecretName(f), namespace, labels, f.Deploy.Annotations, creds.Username, creds.Password, registry) if err != nil { return "", f, fmt.Errorf("problem in creating secret: %v", err) } - err = createAndApplyPipelineRunTemplate(f, namespace, labels) + err = createAndApplyPipelineRunTemplate(pp.kc, f, namespace, labels) if err != nil { return "", f, fmt.Errorf("problem in creating pipeline run: %v", err) } @@ -269,7 +269,7 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn } if newestPipelineRun.Status.GetCondition(apis.ConditionSucceeded).Status == corev1.ConditionFalse { - message := getFailedPipelineRunLog(ctx, client, newestPipelineRun, namespace) + message := getFailedPipelineRunLog(ctx, pp.kc, client, newestPipelineRun, namespace) return "", f, fmt.Errorf("function pipeline run has failed with message: \n\n%s", message) } @@ -451,8 +451,12 @@ func (pp *PipelinesProvider) removeClusterResources(ctx context.Context, f fn.Fu deleteFunctions := []func(context.Context, string, metav1.ListOptions) error{ deletePipelines, deletePipelineRuns, - k8s.DeleteSecrets, - k8s.DeletePersistentVolumeClaims, + func(ctx context.Context, ns string, o metav1.ListOptions) error { + return k8s.DeleteSecrets(ctx, pp.kc, ns, o) + }, + func(ctx context.Context, ns string, o metav1.ListOptions) error { + return k8s.DeletePersistentVolumeClaims(ctx, pp.kc, ns, o) + }, deletePACRepositories, } @@ -544,7 +548,7 @@ out: // getFailedPipelineRunLog returns log message for a failed PipelineRun, // returns log from a container where the failing TaskRun is running, if available. -func getFailedPipelineRunLog(ctx context.Context, client *pipelineClient.TektonV1Client, pr *v1.PipelineRun, namespace string) string { +func getFailedPipelineRunLog(ctx context.Context, kc *k8s.Client, client *pipelineClient.TektonV1Client, pr *v1.PipelineRun, namespace string) string { // Reason "Failed" usually means there is a specific failure in some step, // let's find the failed step and try to get log directly from the container. // If we are not able to get the container's log, we return the generic message from the PipelineRun.Status. @@ -559,7 +563,7 @@ func getFailedPipelineRunLog(ctx context.Context, client *pipelineClient.TektonV for _, s := range t.Status.Steps { // let's try to print logs of the first unsuccessful step if s.Terminated != nil && s.Terminated.ExitCode != 0 { - podLogs, err := k8s.GetPodLogs(ctx, namespace, t.Status.PodName, s.Container) + podLogs, err := k8s.GetPodLogs(ctx, kc, namespace, t.Status.PodName, s.Container) if err == nil { return podLogs } @@ -612,7 +616,7 @@ func findNewestPipelineRunWithRetry(ctx context.Context, f fn.Function, namespac // allows simple mocking in unit tests, use with caution regarding concurrency var createPersistentVolumeClaim = k8s.CreatePersistentVolumeClaim -func createPipelinePersistentVolumeClaim(ctx context.Context, f fn.Function, namespace string, labels map[string]string) error { +func createPipelinePersistentVolumeClaim(ctx context.Context, kc *k8s.Client, f fn.Function, namespace string, labels map[string]string) error { var err error pvcs := DefaultPersistentVolumeClaimSize if f.Build.PVCSize != "" { @@ -620,7 +624,7 @@ func createPipelinePersistentVolumeClaim(ctx context.Context, f fn.Function, nam return fmt.Errorf("PVC size value could not be parsed. %w", err) } } - err = createPersistentVolumeClaim(ctx, getPipelinePvcName(f), namespace, labels, f.Deploy.Annotations, corev1.ReadWriteOnce, pvcs, f.Build.RemoteStorageClass) + err = createPersistentVolumeClaim(ctx, kc, getPipelinePvcName(f), namespace, labels, f.Deploy.Annotations, corev1.ReadWriteOnce, pvcs, f.Build.RemoteStorageClass) if err != nil && !k8serrors.IsAlreadyExists(err) { return fmt.Errorf("problem creating persistent volume claim: %v", err) } diff --git a/pkg/pipelines/tekton/pipelines_provider_test.go b/pkg/pipelines/tekton/pipelines_provider_test.go index 158bc55930..37f5c3c346 100644 --- a/pkg/pipelines/tekton/pipelines_provider_test.go +++ b/pkg/pipelines/tekton/pipelines_provider_test.go @@ -16,6 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" ) func TestSourcesAsTarStream(t *testing.T) { @@ -130,7 +131,7 @@ func TestSourcesAsTarStream_ExcludesFuncDir(t *testing.T) { } func Test_createPipelinePersistentVolumeClaim(t *testing.T) { - type mockType func(ctx context.Context, name, namespaceOverride string, labels map[string]string, annotations map[string]string, accessMode corev1.PersistentVolumeAccessMode, resourceRequest resource.Quantity, storageClass string) (err error) + type mockType func(ctx context.Context, _ *k8s.Client, name, namespaceOverride string, labels map[string]string, annotations map[string]string, accessMode corev1.PersistentVolumeAccessMode, resourceRequest resource.Quantity, storageClass string) (err error) type args struct { ctx context.Context @@ -154,7 +155,7 @@ func Test_createPipelinePersistentVolumeClaim(t *testing.T) { labels: nil, size: DefaultPersistentVolumeClaimSize.String(), }, - mock: func(ctx context.Context, name, namespaceOverride string, labels map[string]string, annotations map[string]string, accessMode corev1.PersistentVolumeAccessMode, resourceRequest resource.Quantity, storageClass string) (err error) { + mock: func(ctx context.Context, _ *k8s.Client, name, namespaceOverride string, labels map[string]string, annotations map[string]string, accessMode corev1.PersistentVolumeAccessMode, resourceRequest resource.Quantity, storageClass string) (err error) { return errors.New("creation of pvc failed") }, wantErr: true, @@ -168,7 +169,7 @@ func Test_createPipelinePersistentVolumeClaim(t *testing.T) { labels: nil, size: DefaultPersistentVolumeClaimSize.String(), }, - mock: func(ctx context.Context, name, namespaceOverride string, labels map[string]string, annotations map[string]string, accessMode corev1.PersistentVolumeAccessMode, resourceRequest resource.Quantity, storageClass string) (err error) { + mock: func(ctx context.Context, _ *k8s.Client, name, namespaceOverride string, labels map[string]string, annotations map[string]string, accessMode corev1.PersistentVolumeAccessMode, resourceRequest resource.Quantity, storageClass string) (err error) { return &apiErrors.StatusError{ErrStatus: metav1.Status{Reason: metav1.StatusReasonAlreadyExists}} }, wantErr: false, @@ -182,7 +183,7 @@ func Test_createPipelinePersistentVolumeClaim(t *testing.T) { labels: nil, size: DefaultPersistentVolumeClaimSize.String(), }, - mock: func(ctx context.Context, name, namespaceOverride string, labels map[string]string, annotations map[string]string, accessMode corev1.PersistentVolumeAccessMode, resourceRequest resource.Quantity, storageClass string) (err error) { + mock: func(ctx context.Context, _ *k8s.Client, name, namespaceOverride string, labels map[string]string, annotations map[string]string, accessMode corev1.PersistentVolumeAccessMode, resourceRequest resource.Quantity, storageClass string) (err error) { return errors.New("no namespace defined") }, wantErr: true, @@ -195,7 +196,7 @@ func Test_createPipelinePersistentVolumeClaim(t *testing.T) { createPersistentVolumeClaim = tt.mock tt.args.f.Build.PVCSize = tt.args.size - if err := createPipelinePersistentVolumeClaim(tt.args.ctx, tt.args.f, tt.args.namespace, tt.args.labels); (err != nil) != tt.wantErr { + if err := createPipelinePersistentVolumeClaim(tt.args.ctx, nil, tt.args.f, tt.args.namespace, tt.args.labels); (err != nil) != tt.wantErr { t.Errorf("createPipelinePersistentVolumeClaim() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/pkg/pipelines/tekton/resources_pac.go b/pkg/pipelines/tekton/resources_pac.go index cff5fa33e3..1398cd3254 100644 --- a/pkg/pipelines/tekton/resources_pac.go +++ b/pkg/pipelines/tekton/resources_pac.go @@ -17,7 +17,7 @@ import ( ) // ensurePACSecretExists checks that up-to-date secret holding credentials needed for PAC is on the cluster -func ensurePACSecretExists(ctx context.Context, f fn.Function, namespace string, credentials pipelines.PacMetadata, labels map[string]string) error { +func ensurePACSecretExists(ctx context.Context, kc *k8s.Client, f fn.Function, namespace string, credentials pipelines.PacMetadata, labels map[string]string) error { dockerConfigJSONContent, err := k8s.HandleDockerCfgJSONContent(credentials.RegistryUsername, credentials.RegistryPassword, "", credentials.RegistryServer) if err != nil { return err @@ -37,7 +37,7 @@ func ensurePACSecretExists(ctx context.Context, f fn.Function, namespace string, secret.Data["provider.token"] = []byte(credentials.PersonalAccessToken) secret.Data["webhook.secret"] = []byte(credentials.WebhookSecret) - return k8s.EnsureSecretExist(ctx, secret, namespace) + return k8s.EnsureSecretExist(ctx, kc, secret, namespace) } // ensurePACRepositoryExists checks that up-to-date Repository CR is present on the cluster diff --git a/pkg/pipelines/tekton/templates.go b/pkg/pipelines/tekton/templates.go index b853194bcf..bdff2588cd 100644 --- a/pkg/pipelines/tekton/templates.go +++ b/pkg/pipelines/tekton/templates.go @@ -97,7 +97,7 @@ type templateData struct { // createPipelineTemplatePAC creates a Pipeline template used for PAC on-cluster build // it creates the resource in the project directory -func createPipelineTemplatePAC(f fn.Function, labels map[string]string) error { +func createPipelineTemplatePAC(kc *k8s.Client, f fn.Function, labels map[string]string) error { // Determine if TLS verification should be skipped tlsVerify := "true" if f.RegistryInsecure || isInsecureRegistry(f.Registry) { @@ -142,7 +142,7 @@ func createPipelineTemplatePAC(f fn.Function, labels map[string]string) error { // createPipelineRunTemplatePAC creates a PipelineRun template used for PAC on-cluster build // it creates the resource in the project directory -func createPipelineRunTemplatePAC(f fn.Function, labels map[string]string) error { +func createPipelineRunTemplatePAC(kc *k8s.Client, f fn.Function, labels map[string]string) error { contextDir := f.Build.Git.ContextDir if contextDir == "" && f.Build.Builder == builders.S2I { // TODO(lkingland): could instead update S2I to interpret empty string @@ -301,7 +301,7 @@ func getTaskSpec(taskYaml string) (string, error) { // createAndApplyPipelineTemplate creates and applies Pipeline template for a standard on-cluster build // all resources are created on the fly, if there's a Pipeline defined in the project directory, it is used instead -func createAndApplyPipelineTemplate(f fn.Function, namespace string, labels map[string]string) error { +func createAndApplyPipelineTemplate(kc *k8s.Client, f fn.Function, namespace string, labels map[string]string) error { // If Git is set up create fetch task and reference it from build task, // otherwise sources have been already uploaded to workspace PVC. @@ -344,12 +344,12 @@ func createAndApplyPipelineTemplate(f fn.Function, namespace string, labels map[ return builders.ErrBuilderNotSupported{Builder: f.Build.Builder} } - return createAndApplyResource(f.Root, pipelineFileName, template, "pipeline", getPipelineName(f), namespace, data) + return createAndApplyResource(kc, f.Root, pipelineFileName, template, "pipeline", getPipelineName(f), namespace, data) } // createAndApplyPipelineRunTemplate creates and applies PipelineRun template for a standard on-cluster build // all resources are created on the fly, if there's a PipelineRun defined in the project directory, it is used instead -func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels map[string]string) error { +func createAndApplyPipelineRunTemplate(kc *k8s.Client, f fn.Function, namespace string, labels map[string]string) error { contextDir := f.Build.Git.ContextDir if contextDir == "" && f.Build.Builder == builders.S2I { // TODO(lkingland): could instead update S2I to interpret empty string @@ -424,7 +424,7 @@ func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels m return builders.ErrBuilderNotSupported{Builder: f.Build.Builder} } - return createAndApplyResource(f.Root, pipelineFileName, template, "pipelinerun", getPipelineRunGenerateName(f), namespace, data) + return createAndApplyResource(kc, f.Root, pipelineFileName, template, "pipelinerun", getPipelineRunGenerateName(f), namespace, data) } // allows simple mocking in unit tests @@ -432,7 +432,7 @@ var manifestivalClient = k8s.GetManifestivalClient // createAndApplyResource tries to create and apply a resource to the k8s cluster from the input template and data, // if there's the same resource already created in the project directory, it is used instead -func createAndApplyResource(projectRoot, fileName, fileTemplate, kind, resourceName, namespace string, data interface{}) error { +func createAndApplyResource(kc *k8s.Client, projectRoot, fileName, fileTemplate, kind, resourceName, namespace string, data interface{}) error { var source manifestival.Source filePath := path.Join(projectRoot, resourcesDirectory, fileName) @@ -452,7 +452,7 @@ func createAndApplyResource(projectRoot, fileName, fileTemplate, kind, resourceN source = manifestival.Reader(&buf) } - client, err := manifestivalClient() + client, err := manifestivalClient(kc) if err != nil { return fmt.Errorf("error generating template: %v", err) } diff --git a/pkg/pipelines/tekton/templates_int_test.go b/pkg/pipelines/tekton/templates_int_test.go index 406c9e746c..69b0f2fb69 100644 --- a/pkg/pipelines/tekton/templates_int_test.go +++ b/pkg/pipelines/tekton/templates_int_test.go @@ -9,6 +9,7 @@ import ( "github.com/manifestival/manifestival/fake" fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" . "knative.dev/func/pkg/testing" ) @@ -19,7 +20,7 @@ func TestInt_createAndApplyPipelineTemplate(t *testing.T) { old := manifestivalClient defer func() { manifestivalClient = old }() - manifestivalClient = func() (manifestival.Client, error) { + manifestivalClient = func(*k8s.Client) (manifestival.Client, error) { return fake.New(), nil } @@ -36,7 +37,7 @@ func TestInt_createAndApplyPipelineTemplate(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - if err := createAndApplyPipelineTemplate(f, tt.namespace, tt.labels); (err != nil) != tt.wantErr { + if err := createAndApplyPipelineTemplate(k8s.NewClientFromKubeconfig(), f, tt.namespace, tt.labels); (err != nil) != tt.wantErr { t.Errorf("createAndApplyPipelineTemplate() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/pkg/pipelines/tekton/templates_test.go b/pkg/pipelines/tekton/templates_test.go index 7523ce931c..0147f88832 100644 --- a/pkg/pipelines/tekton/templates_test.go +++ b/pkg/pipelines/tekton/templates_test.go @@ -14,6 +14,7 @@ import ( "knative.dev/func/pkg/builders" fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" . "knative.dev/func/pkg/testing" ) @@ -90,7 +91,7 @@ func Test_createPipelineTemplatePAC(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - err = createPipelineTemplatePAC(f, make(map[string]string)) + err = createPipelineTemplatePAC(nil, f, make(map[string]string)) if (err != nil) != tt.wantErr { t.Errorf("createPipelineTemplate() error = %v, wantErr %v", err, tt.wantErr) @@ -152,7 +153,7 @@ func Test_createPipelineRunTemplatePAC(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - err = createPipelineRunTemplatePAC(f, make(map[string]string)) + err = createPipelineRunTemplatePAC(nil, f, make(map[string]string)) if (err != nil) != tt.wantErr { t.Errorf("createPipelineRunTemplate() error = %v, wantErr %v", err, tt.wantErr) @@ -304,7 +305,7 @@ func Test_createAndApplyPipelineRunTemplate(t *testing.T) { old := manifestivalClient defer func() { manifestivalClient = old }() - manifestivalClient = func() (manifestival.Client, error) { + manifestivalClient = func(*k8s.Client) (manifestival.Client, error) { return fake.New(), nil } @@ -321,7 +322,7 @@ func Test_createAndApplyPipelineRunTemplate(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - if err := createAndApplyPipelineRunTemplate(f, tt.namespace, tt.labels); (err != nil) != tt.wantErr { + if err := createAndApplyPipelineRunTemplate(nil, f, tt.namespace, tt.labels); (err != nil) != tt.wantErr { t.Errorf("createAndApplyPipelineRunTemplate() error = %v, wantErr %v", err, tt.wantErr) } }) From 218f79a501150c7d0cc59d3f9e033ef20787033a Mon Sep 17 00:00:00 2001 From: gauron99 Date: Fri, 28 Aug 2026 21:37:40 +0200 Subject: [PATCH 3/3] Inject k8s.Client into tekton, operator and config; remove k8s globals Finish the client injection. Every cluster-facing component now receives its *k8s.Client from its constructor or caller; nothing below the command layer reads the kubeconfig on its own. - tekton: NewTektonClient and NewTektonClients take the client; the PipelinesProvider passes its own to pipeline, pipeline-run and PAC repository deletion. - pac: NewTektonPacClientAndResolvedNamespace, DetectPACInstallation, DetectPACOpenShiftRoute and GetPACInfo take the client. - operator: NewSyncer takes the client and passes it to SyncFunctionCR. - config: Global.RegistryDefault takes the client for OpenShift detection. - cmd: the remaining direct readers (defaultNamespace, registry, environment, completion, deploy messages) resolve the client explicitly or receive the command's client. With no callers left, the package-level GetClientConfig, NewKubernetesClientset, NewDynamicClient, NewClientAndResolvedNamespace, GetDefaultNamespace, GetDefaultOpenShiftRegistry and IsOpenShift are removed together with the process-wide OpenShift detection cache. The kubeconfig loading rules live in one unexported function behind NewClientFromKubeconfig. Integration and e2e tests construct their client with NewClientFromKubeconfig at the top of the test. --- cmd/client.go | 14 +- cmd/completion_util.go | 2 +- cmd/deploy.go | 6 +- cmd/deploy_test.go | 2 +- cmd/environment.go | 2 +- cmd/logs.go | 16 +- cmd/logs_test.go | 16 +- cmd/root.go | 4 +- e2e/e2e_core_test.go | 2 +- e2e/e2e_expose_test.go | 18 +- e2e/e2e_test.go | 4 +- pkg/builders/builders_int_test.go | 2 +- pkg/config/config.go | 14 +- .../testing/integration_test_helper.go | 22 +-- pkg/functions/client_int_test.go | 6 +- pkg/http/openshift.go | 7 +- pkg/http/transport.go | 13 +- pkg/http/transport_test.go | 20 +++ pkg/k8s/client.go | 170 ++++++++---------- pkg/k8s/client_test.go | 111 ++++++++++-- pkg/k8s/deployer_int_test.go | 16 +- pkg/k8s/describer_int_test.go | 2 +- pkg/k8s/dialer.go | 1 - pkg/k8s/dialer_int_test.go | 16 +- pkg/k8s/lister_int_test.go | 2 +- pkg/k8s/logs_int_test.go | 4 +- pkg/k8s/openshift.go | 72 -------- pkg/k8s/persistent_volumes.go | 1 - pkg/k8s/remover_int_test.go | 2 +- pkg/keda/deployer.go | 3 +- pkg/keda/deployer_int_test.go | 16 +- pkg/keda/describer_int_test.go | 2 +- pkg/keda/exposure.go | 9 +- pkg/keda/exposure_test.go | 18 +- pkg/keda/lister_int_test.go | 2 +- pkg/keda/remover.go | 2 +- pkg/keda/remover_int_test.go | 2 +- pkg/knative/deployer_int_test.go | 16 +- pkg/knative/describer_int_test.go | 2 +- pkg/knative/lister_int_test.go | 2 +- pkg/knative/remover_int_test.go | 2 +- pkg/operator/sync.go | 5 +- pkg/operator/syncer.go | 13 +- pkg/pipelines/tekton/client.go | 10 +- pkg/pipelines/tekton/gitlab_int_test.go | 6 +- pkg/pipelines/tekton/pac/client.go | 6 +- pkg/pipelines/tekton/pac/pac.go | 14 +- pkg/pipelines/tekton/pipelines_int_test.go | 6 +- .../tekton/pipelines_pac_provider.go | 16 +- .../tekton/pipelines_pac_provider_test.go | 7 +- pkg/pipelines/tekton/pipelines_provider.go | 33 ++-- pkg/pipelines/tekton/resources.go | 9 +- pkg/pipelines/tekton/resources_pac.go | 8 +- pkg/testing/k8s/testing.go | 2 +- 54 files changed, 390 insertions(+), 388 deletions(-) diff --git a/cmd/client.go b/cmd/client.go index f6562b87a4..ff491ad007 100644 --- a/cmd/client.go +++ b/cmd/client.go @@ -93,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))), } ) @@ -124,14 +124,7 @@ func newK8sClient(kc *k8s.Client) *k8s.Client { // newTransport returns a transport with cluster-flavor-specific variations // which take advantage of additional features offered by cluster variants. func newTransport(kc *k8s.Client, insecureSkipVerify bool) fnhttp.RoundTripCloser { - opts := []fnhttp.Option{ - fnhttp.WithInsecureSkipVerify(insecureSkipVerify), - fnhttp.WithOpenShiftServiceCA(kc), - } - if kc != nil && kc.Loader() != nil { - opts = append(opts, fnhttp.WithInClusterDialer(k8s.NewLazyInitInClusterDialer(kc))) - } - return fnhttp.NewRoundTripper(kc, opts...) + return fnhttp.NewRoundTripper(kc, fnhttp.WithInsecureSkipVerify(insecureSkipVerify), fnhttp.WithOpenShiftServiceCA(kc)) } // newCredentialsProvider returns a credentials provider which possibly @@ -183,10 +176,9 @@ func newTektonPipelinesProvider(kc *k8s.Client, creds oci.CredentialsProvider, v tekton.WithVerbose(verbose), tekton.WithPipelineDecorator(deployDecorator{kc}), tekton.WithTransport(transport), - tekton.WithK8sClient(kc), } - return tekton.NewPipelinesProvider(options...) + return tekton.NewPipelinesProvider(kc, options...) } func newKnativeDeployer(kc *k8s.Client, verbose bool) fn.Deployer { diff --git a/cmd/completion_util.go b/cmd/completion_util.go index 60ca1c8bb7..ec79d37d8d 100644 --- a/cmd/completion_util.go +++ b/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), diff --git a/cmd/deploy.go b/cmd/deploy.go index a6ad16a662..89745dc711 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -357,7 +357,7 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { } // Informative non-error messages regarding the final deployment request - printDeployMessages(cmd.OutOrStdout(), f) + printDeployMessages(cmd.OutOrStdout(), kc, f) // create client with options from cfg clientOptions, err := cfg.clientOptions(kc) @@ -890,7 +890,7 @@ func (c deployConfig) clientOptions(kc *k8s.Client) ([]fn.Option, error) { } // printDeployMessages to the output. Non-error deployment messages. -func printDeployMessages(out io.Writer, f fn.Function) { +func printDeployMessages(out io.Writer, kc *k8s.Client, f fn.Function) { digest, err := isDigested(f.Image) if err == nil && digest { fmt.Fprintf(out, "Deploying image '%v', which has a digest. Build and push are disabled.\n", f.Image) @@ -915,7 +915,7 @@ func printDeployMessages(out io.Writer, f fn.Function) { // If the target namespace is provided but differs from active, warn because // the function won't be visible to other commands such as kubectl unless // context namespace is switched. - activeNamespace, err := k8s.GetDefaultNamespace() + activeNamespace, err := kc.DefaultNamespace() if err == nil && targetNamespace != "" && targetNamespace != activeNamespace { fmt.Fprintf(out, "Warning: namespace chosen is '%s', but currently active namespace is '%s'. Continuing with deployment to '%s'.\n", targetNamespace, activeNamespace, targetNamespace) } diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index 0d7e9b45cb..78eb8b877d 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -1239,7 +1239,7 @@ func TestDeploy_NamespaceUpdateWarning(t *testing.T) { time.Sleep(1 * time.Second) - activeNamespace, err := k8s.GetDefaultNamespace() + activeNamespace, err := k8s.NewClientFromKubeconfig().DefaultNamespace() if err != nil { t.Fatalf("Couldn't get active namespace, got error: %v", err) } diff --git a/cmd/environment.go b/cmd/environment.go index 31aee04f31..c7adeefd5b 100644 --- a/cmd/environment.go +++ b/cmd/environment.go @@ -116,7 +116,7 @@ func runEnvironment(cmd *cobra.Command, newClient ClientFactory, v *Version) (er // Gets the cluster host var host string - cc, err := k8s.GetClientConfig().ClientConfig() + cc, err := k8s.NewClientFromKubeconfig().RestConfig() if err != nil { fmt.Printf("error getting client config %v\n", err) } else { diff --git a/cmd/logs.go b/cmd/logs.go index e75bd940e4..323fd874c9 100644 --- a/cmd/logs.go +++ b/cmd/logs.go @@ -19,13 +19,12 @@ import ( "knative.dev/func/pkg/knative" ) -// logGatherer writes the logs of a deployed function. -type logGatherer func(context.Context, knative.LogsOptions, io.Writer) error +// logGatherer writes the logs of a deployed function, reading them through +// the given cluster client. +type logGatherer func(context.Context, *k8s.Client, knative.LogsOptions, io.Writer) error func NewLogsCmd(newClient ClientFactory) *cobra.Command { - return newLogsCmd(newClient, func(ctx context.Context, opts knative.LogsOptions, out io.Writer) error { - return knative.GetKServiceLogs(ctx, k8s.NewClientFromKubeconfig(), opts, out) - }) + return newLogsCmd(newClient, knative.GetKServiceLogs) } // newLogsCmd constructs the command with an explicit log gatherer, allowing @@ -101,7 +100,10 @@ func runLogs(cmd *cobra.Command, newClient ClientFactory, gather logGatherer) er return err } - client, done := newClient(ClientConfig{Verbose: cfg.Verbose}) + // The cluster client for this command, shared by the describer (through + // newClient) and the log gatherer. + kc := k8s.NewClientFromKubeconfig() + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, K8sClient: kc}) defer done() // Get function details and deployer type @@ -199,7 +201,7 @@ func runLogs(cmd *cobra.Command, newClient ClientFactory, gather logGatherer) er // filter pods. It is that of the latest revision, so filtering on it // would drop the logs of the pods actually serving traffic during a // rollout. All pods of the function's service are of interest here. - err = gather(ctx, knative.LogsOptions{ + err = gather(ctx, kc, knative.LogsOptions{ Name: f.Name, Namespace: f.Namespace, Since: sinceTime, diff --git a/cmd/logs_test.go b/cmd/logs_test.go index 08839d839c..cd899a8af1 100644 --- a/cmd/logs_test.go +++ b/cmd/logs_test.go @@ -161,7 +161,7 @@ func TestLogs_DefaultSnapshot(t *testing.T) { var opts knative.LogsOptions stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{} - cmd := newTestLogsCmd(func(ctx context.Context, o knative.LogsOptions, out io.Writer) error { + cmd := newTestLogsCmd(func(ctx context.Context, _ *k8s.Client, o knative.LogsOptions, out io.Writer) error { opts = o if o.Follow { // would block indefinitely in the real implementation <-ctx.Done() @@ -207,7 +207,7 @@ func TestLogs_Follow(t *testing.T) { defer cancel() stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{} - cmd := newTestLogsCmd(func(ctx context.Context, o knative.LogsOptions, out io.Writer) error { + cmd := newTestLogsCmd(func(ctx context.Context, _ *k8s.Client, o knative.LogsOptions, out io.Writer) error { opts = o close(started) <-ctx.Done() @@ -270,7 +270,7 @@ func TestLogs_Window(t *testing.T) { _ = FromTempDirectory(t) var opts knative.LogsOptions - cmd := newTestLogsCmd(func(_ context.Context, o knative.LogsOptions, _ io.Writer) error { + cmd := newTestLogsCmd(func(_ context.Context, _ *k8s.Client, o knative.LogsOptions, _ io.Writer) error { opts = o return nil }, &bytes.Buffer{}, &bytes.Buffer{}) @@ -308,7 +308,7 @@ func TestLogs_NoPods(t *testing.T) { _ = FromTempDirectory(t) stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{} - cmd := newTestLogsCmd(func(_ context.Context, _ knative.LogsOptions, _ io.Writer) error { + cmd := newTestLogsCmd(func(_ context.Context, _ *k8s.Client, _ knative.LogsOptions, _ io.Writer) error { return k8s.ErrNoMatchingPods }, stdout, stderr) cmd.SetArgs([]string{"--name", "myfunc"}) @@ -330,7 +330,7 @@ func TestLogs_PartialLogs(t *testing.T) { _ = FromTempDirectory(t) stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{} - cmd := newTestLogsCmd(func(_ context.Context, _ knative.LogsOptions, out io.Writer) error { + cmd := newTestLogsCmd(func(_ context.Context, _ *k8s.Client, _ knative.LogsOptions, out io.Writer) error { _, _ = out.Write([]byte("log line\n")) return &k8s.PartialLogsError{Err: errors.New("pod is terminating")} }, stdout, stderr) @@ -351,7 +351,7 @@ func TestLogs_PartialLogs(t *testing.T) { func TestLogs_Failure(t *testing.T) { _ = FromTempDirectory(t) - cmd := newTestLogsCmd(func(_ context.Context, _ knative.LogsOptions, _ io.Writer) error { + cmd := newTestLogsCmd(func(_ context.Context, _ *k8s.Client, _ knative.LogsOptions, _ io.Writer) error { return errors.New("forbidden") }, &bytes.Buffer{}, &bytes.Buffer{}) cmd.SetArgs([]string{"--name", "myfunc"}) @@ -369,7 +369,7 @@ func TestLogs_ImageNotFiltered(t *testing.T) { _ = FromTempDirectory(t) var opts knative.LogsOptions - cmd := newTestLogsCmd(func(_ context.Context, o knative.LogsOptions, _ io.Writer) error { + cmd := newTestLogsCmd(func(_ context.Context, _ *k8s.Client, o knative.LogsOptions, _ io.Writer) error { opts = o return nil }, &bytes.Buffer{}, &bytes.Buffer{}) @@ -394,7 +394,7 @@ func TestLogs_FollowEnv(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - cmd := newTestLogsCmd(func(ctx context.Context, o knative.LogsOptions, _ io.Writer) error { + cmd := newTestLogsCmd(func(ctx context.Context, _ *k8s.Client, o knative.LogsOptions, _ io.Writer) error { opts = o cancel() return ctx.Err() diff --git a/cmd/root.go b/cmd/root.go index ec1e2e680f..dd0761d0f1 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -145,7 +145,7 @@ func registry() string { return r } cfg, _ := config.NewDefault() - return cfg.RegistryDefault() + return cfg.RegistryDefault(k8s.NewClientFromKubeconfig()) } // effectivePath to use is that which was provided by --path or FUNC_PATH. @@ -190,7 +190,7 @@ func defaultNamespace(f fn.Function, verbose bool) string { } // Active K8S namespace - namespace, err := k8s.GetDefaultNamespace() + namespace, err := k8s.NewClientFromKubeconfig().DefaultNamespace() if err != nil { if verbose { fmt.Fprintf(os.Stderr, "Unable to get current active kubernetes namespace. Defaults will be used. %v", err) diff --git a/e2e/e2e_core_test.go b/e2e/e2e_core_test.go index 70811e6646..3a6a30fbb9 100644 --- a/e2e/e2e_core_test.go +++ b/e2e/e2e_core_test.go @@ -399,7 +399,7 @@ func TestCore_Delete(t *testing.T) { } // Check it appears in the list - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() client := fn.New(fn.WithListers(knative.NewLister(kc, false))) list, err := client.List(t.Context(), Namespace) if err != nil { diff --git a/e2e/e2e_expose_test.go b/e2e/e2e_expose_test.go index 85a84ec286..f811045c5f 100644 --- a/e2e/e2e_expose_test.go +++ b/e2e/e2e_expose_test.go @@ -67,7 +67,7 @@ func requiresOpenShift(t *testing.T) { // IsOpenShift caches its first answer for the whole binary. setupEnv sets // this same value later; the probe needs it first. os.Setenv("KUBECONFIG", Kubeconfig) - if !k8s.IsOpenShift() { + if ok, _ := k8s.NewClientFromKubeconfig().IsOpenShift(); !ok { t.Skip("not an OpenShift cluster: route.openshift.io is unavailable, " + "so there is no Route to assert on") } @@ -79,7 +79,7 @@ func requiresOpenShift(t *testing.T) { func requiresNotOpenShift(t *testing.T) { t.Helper() os.Setenv("KUBECONFIG", Kubeconfig) // see requiresOpenShift - if k8s.IsOpenShift() { + if ok, _ := k8s.NewClientFromKubeconfig().IsOpenShift(); ok { t.Skip("this is an OpenShift cluster, where a Route is possible, " + "so there is no refusal to assert on") } @@ -423,7 +423,7 @@ func TestExpose_KedaRoute(t *testing.T) { // the identity rules change. func routeCount(t *testing.T, ns, fnName, fnNamespace string) int { t.Helper() - client, err := k8s.NewDynamicClient() + client, err := k8s.NewClientFromKubeconfig().DynamicClient() if err != nil { t.Fatal(err) } @@ -444,7 +444,7 @@ func routeCount(t *testing.T, ns, fnName, fnNamespace string) int { // records the exposed hostname and the Route's namespace. func serviceAnnotations(t *testing.T, ns, name string) map[string]string { t.Helper() - clientset, err := k8s.NewKubernetesClientset() + clientset, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } @@ -560,7 +560,7 @@ func TestExpose_KedaDeleteCleansRoute(t *testing.T) { if n := routeCount(t, recordedNS, name, ns); n != 0 { t.Errorf("expected delete to remove the Route, found %d left in %q", n, recordedNS) } - clientset, err := k8s.NewKubernetesClientset() + clientset, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } @@ -627,7 +627,7 @@ func TestExpose_KedaRouteDomain(t *testing.T) { // routeFor returns the one Route carrying this function's identity labels. func routeFor(t *testing.T, ns, fnName, fnNamespace string) *unstructured.Unstructured { t.Helper() - client, err := k8s.NewDynamicClient() + client, err := k8s.NewClientFromKubeconfig().DynamicClient() if err != nil { t.Fatal(err) } @@ -735,7 +735,7 @@ func TestExpose_RouteDomainTLS(t *testing.T) { // Play the certificate controller: inject a self-signed cert for the // domain, exactly as cert-manager's openshift-routes plugin would. certPEM, keyPEM, pool := selfSignedCert(t, domain) - client, err := k8s.NewDynamicClient() + client, err := k8s.NewClientFromKubeconfig().DynamicClient() if err != nil { t.Fatal(err) } @@ -900,7 +900,7 @@ func liveInterceptorNamespace(t *testing.T) string { t.Helper() // Same Service keda.interceptorNamespace looks for. const interceptorServiceName = "keda-add-ons-http-interceptor-proxy" - clientset, err := k8s.NewKubernetesClientset() + clientset, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } @@ -922,7 +922,7 @@ var grantPipelineInterceptorOnce sync.Once func grantPipelineSAInterceptorAccess(t *testing.T, interceptorNS string) { t.Helper() grantPipelineInterceptorOnce.Do(func() { - clientset, err := k8s.NewKubernetesClientset() + clientset, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index 2039c6cc61..fcb13a1709 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -888,7 +888,7 @@ func isAbnormalExit(t *testing.T, err error) bool { func setSecret(t *testing.T, name, ns string, data map[string][]byte) { t.Helper() ctx := t.Context() - config, err := k8s.GetClientConfig().ClientConfig() + config, err := k8s.NewClientFromKubeconfig().RestConfig() if err != nil { t.Fatal(err) } @@ -911,7 +911,7 @@ func setSecret(t *testing.T, name, ns string, data map[string][]byte) { func setConfigMap(t *testing.T, name, ns string, data map[string]string) { t.Helper() ctx := t.Context() - config, err := k8s.GetClientConfig().ClientConfig() + config, err := k8s.NewClientFromKubeconfig().RestConfig() if err != nil { t.Fatal(err) } diff --git a/pkg/builders/builders_int_test.go b/pkg/builders/builders_int_test.go index a57fbd58c0..461980a6b8 100644 --- a/pkg/builders/builders_int_test.go +++ b/pkg/builders/builders_int_test.go @@ -374,7 +374,7 @@ func servePrivateGit(ctx context.Context, t *testing.T, certDir string) { image = "ghcr.io/matejvasek/git-private:latest" ) - k8sClient, err := k8s.NewKubernetesClientset() + k8sClient, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } diff --git a/pkg/config/config.go b/pkg/config/config.go index 00d7e180a2..2d720310b2 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -61,19 +61,17 @@ func New() Global { } // RegistryDefault is a convenience method for deferred calculation of a -// default registry taking into account both the global config file and cluster -// detection. -func (c Global) RegistryDefault() string { +// default registry taking into account both the global config file and the +// cluster kc points at. +func (c Global) RegistryDefault(kc *k8s.Client) string { // If defined, the user's choice for global registry default value is used if c.Registry != "" { return c.Registry } - switch { - case k8s.IsOpenShift(): - return k8s.GetDefaultOpenShiftRegistry() - default: - return "" + if ok, _ := kc.IsOpenShift(); ok { + return kc.DefaultOpenShiftRegistry() } + return "" } // NewDefault returns a config populated by global defaults as defined by the diff --git a/pkg/deployer/testing/integration_test_helper.go b/pkg/deployer/testing/integration_test_helper.go index 4cefbfa5ce..b0f71e208f 100644 --- a/pkg/deployer/testing/integration_test_helper.go +++ b/pkg/deployer/testing/integration_test_helper.go @@ -465,7 +465,7 @@ func TestInt_Scale(t *testing.T, deployer fn.Deployer, remover fn.Remover, descr // Check the actual number of pods running using Kubernetes API // This is much more reliable than checking logs - cliSet, err := k8s.NewKubernetesClientset() + cliSet, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } @@ -616,7 +616,7 @@ func TestInt_EnvsUpdate(t *testing.T, deployer fn.Deployer, remover fn.Remover, t.Fatal(err) } - cliSet, err := k8s.NewKubernetesClientset() + cliSet, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } @@ -671,7 +671,7 @@ func TestInt_FullPath(t *testing.T, deployer fn.Deployer, remover fn.Remover, li ctx, cancel := context.WithTimeout(context.Background(), time.Minute*10) t.Cleanup(cancel) - cliSet, err := k8s.NewKubernetesClientset() + cliSet, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } @@ -1064,7 +1064,7 @@ func createTrigger(t *testing.T, ctx context.Context, namespace, triggerName str }, }, } - eventingClient, err := knative.NewEventingClient(k8s.NewClient(k8s.GetClientConfig()), namespace) + eventingClient, err := knative.NewEventingClient(k8s.NewClientFromKubeconfig(), namespace) if err != nil { t.Fatal(err) } @@ -1097,7 +1097,7 @@ func createTrigger(t *testing.T, ctx context.Context, namespace, triggerName str func createSecret(t *testing.T, namespace, name string, data map[string]string) { t.Helper() - cliSet, err := k8s.NewKubernetesClientset() + cliSet, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } @@ -1128,7 +1128,7 @@ func createSecret(t *testing.T, namespace, name string, data map[string]string) func createConfigMap(t *testing.T, namespace, name string, data map[string]string) { t.Helper() - cliSet, err := k8s.NewKubernetesClientset() + cliSet, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } @@ -1155,19 +1155,19 @@ func deferCleanup(t *testing.T, namespace string, resourceType string, name stri switch resourceType { case "secret": t.Cleanup(func() { - if cliSet, err := k8s.NewKubernetesClientset(); err == nil { + if cliSet, err := k8s.NewClientFromKubeconfig().Clientset(); err == nil { _ = cliSet.CoreV1().Secrets(namespace).Delete(context.Background(), name, metav1.DeleteOptions{}) } }) case "configmap": t.Cleanup(func() { - if cliSet, err := k8s.NewKubernetesClientset(); err == nil { + if cliSet, err := k8s.NewClientFromKubeconfig().Clientset(); err == nil { _ = cliSet.CoreV1().ConfigMaps(namespace).Delete(context.Background(), name, metav1.DeleteOptions{}) } }) case "trigger": t.Cleanup(func() { - if eventingClient, err := knative.NewEventingClient(k8s.NewClient(k8s.GetClientConfig()), namespace); err == nil { + if eventingClient, err := knative.NewEventingClient(k8s.NewClientFromKubeconfig(), namespace); err == nil { _ = eventingClient.DeleteTrigger(context.Background(), name) } }) @@ -1248,7 +1248,7 @@ func TestInt_OperatorSync(t *testing.T, deployer fn.Deployer, remover fn.Remover fn.WithDeployer(deployer), fn.WithDescribers(describer), fn.WithRemovers(remover), - fn.WithSyncer(operator.NewSyncer()), + fn.WithSyncer(operator.NewSyncer(k8s.NewClientFromKubeconfig())), ) f, err := client.Init(fn.Function{ @@ -1295,7 +1295,7 @@ func TestInt_OperatorSync(t *testing.T, deployer fn.Deployer, remover fn.Remover }) // Verify CR state only if the Function CRD is installed - restCfg, err := k8s.GetClientConfig().ClientConfig() + restCfg, err := k8s.NewClientFromKubeconfig().RestConfig() if err != nil { t.Fatalf("getting kubernetes config: %v", err) } diff --git a/pkg/functions/client_int_test.go b/pkg/functions/client_int_test.go index e028665cdd..a5ce4fc8a2 100644 --- a/pkg/functions/client_int_test.go +++ b/pkg/functions/client_int_test.go @@ -206,7 +206,7 @@ func TestInt_Deploy_WithTriggers(t *testing.T) { } func TestInt_Update_WithAnnotationsAndLabels(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() resetEnv() _, cleanup := Mktemp(t) defer cleanup() @@ -667,7 +667,7 @@ func resetEnv() { // newClient creates an instance of the func client with concrete impls // sufficient for running integration tests. func newClient(verbose bool) *fn.Client { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() return fn.New( fn.WithRegistry(DefaultIntTestRegistry), fn.WithRegistryInsecure(true), @@ -684,7 +684,7 @@ func newClient(verbose bool) *fn.Client { // copy of newClient just with s2i methods instead func newClientWithS2i(verbose bool) *fn.Client { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() return fn.New( fn.WithRegistry(DefaultIntTestRegistry), fn.WithRegistryInsecure(true), diff --git a/pkg/http/openshift.go b/pkg/http/openshift.go index 7c6be7104b..229f5afa29 100644 --- a/pkg/http/openshift.go +++ b/pkg/http/openshift.go @@ -12,8 +12,13 @@ import ( const openShiftRegistryHost = "image-registry.openshift-image-registry.svc" -// WithOpenShiftServiceCA enables trust to OpenShift's service CA for internal image registry +// WithOpenShiftServiceCA enables trust to OpenShift's service CA for the +// internal image registry. Without a cluster client there is no way to fetch +// the CA, so the option does nothing. func WithOpenShiftServiceCA(c *k8s.Client) Option { + if c == nil { + return func(*options) {} + } var err error var ca *x509.Certificate var o sync.Once diff --git a/pkg/http/transport.go b/pkg/http/transport.go index 8947ed8cc6..87ed9d9f23 100644 --- a/pkg/http/transport.go +++ b/pkg/http/transport.go @@ -63,7 +63,9 @@ func NewRoundTripper(kc *k8s.Client, opts ...Option) RoundTripCloser { for _, option := range opts { option(&o) } - if o.inClusterDialer == nil { + // Without a cluster client there is no pod to dial from; the transport + // then dials directly only. + if o.inClusterDialer == nil && kc != nil { o.inClusterDialer = k8s.NewLazyInitInClusterDialer(kc) } @@ -133,7 +135,7 @@ func (d *dialerWithFallback) DialContext(ctx context.Context, network, address s } var dnsErr *net.DNSError - if !errors.As(err, &dnsErr) { + if !errors.As(err, &dnsErr) || d.fallbackDialer == nil { return nil, err } @@ -149,9 +151,10 @@ func (d *dialerWithFallback) Close() error { errs = append(errs, err) } - err = d.fallbackDialer.Close() - if err != nil { - errs = append(errs, err) + if d.fallbackDialer != nil { + if err = d.fallbackDialer.Close(); err != nil { + errs = append(errs, err) + } } if len(errs) > 0 { diff --git a/pkg/http/transport_test.go b/pkg/http/transport_test.go index 8d02db76b4..22f956eb1d 100644 --- a/pkg/http/transport_test.go +++ b/pkg/http/transport_test.go @@ -148,3 +148,23 @@ func startServer(t *testing.T, hostname string) (addr string, ca *x509.Certifica }() return } + +// TestNilClient_TransportDialsDirectly ensures a transport built without a +// cluster client still works as a plain transport: a hostname that does not +// resolve yields a dial error, not a panic in a missing in-cluster fallback +// dialer or a missing service CA client. +func TestNilClient_TransportDialsDirectly(t *testing.T) { + transport := fnhttp.NewRoundTripper(nil, fnhttp.WithOpenShiftServiceCA(nil)) + defer transport.Close() + + client := http.Client{Transport: transport} + + // .invalid is reserved and never resolves; this is the case that would + // otherwise fall back to the in-cluster dialer. The OpenShift registry + // host is used so the service CA path is exercised too. + resp, err := client.Get("https://image-registry.openshift-image-registry.svc.invalid:5000/v2/") + if err == nil { + _ = resp.Body.Close() + t.Fatal("expected a dial error for an unresolvable host") + } +} diff --git a/pkg/k8s/client.go b/pkg/k8s/client.go index f8b15bf785..3658e1b2a5 100644 --- a/pkg/k8s/client.go +++ b/pkg/k8s/client.go @@ -1,6 +1,7 @@ package k8s import ( + "errors" "fmt" "sync" "time" @@ -23,83 +24,95 @@ const ( // once, at the top of the call chain (the CLI), and passed down to every // component which talks to the cluster. Components must NOT construct their // own cluster configuration. +// +// A Client is backed by exactly one of: +// - a kubeconfig loader (NewClient, NewClientFromKubeconfig), which also +// knows contexts and the active namespace; +// - a bare rest.Config (NewClientFromConfig), for callers such as services +// which hold a host and a token and have no kubeconfig at all. type Client struct { + // cc is the kubeconfig loader: merged files, contexts, active namespace. + // Nil when the Client was built from a rest.Config. cc clientcmd.ClientConfig - cfg *rest.Config - cfgErr error - cfgOnce sync.Once + // cfg is the rest configuration given to NewClientFromConfig. + // Nil when the Client was built from a kubeconfig loader. + cfg *rest.Config - // isOpenShift answers IsOpenShift: memoized detection, or a preset. + // isOpenShift answers IsOpenShift; detection runs once per Client. isOpenShift func() (bool, error) } -// ClientOpt configures a Client. -type ClientOpt func(*Client) +// errNoKubeconfig is returned by the methods which need a kubeconfig when the +// Client was built from a bare rest.Config. +var errNoKubeconfig = errors.New("client was built from a rest config; no kubeconfig available") -// WithOpenShift sets the OpenShift detection result up front, so the Client -// never contacts the cluster to find out. Intended for tests. -func WithOpenShift(v bool) ClientOpt { - return func(c *Client) { - c.isOpenShift = func() (bool, error) { return v, nil } - } -} +// errNoConfiguration is returned when a Client was built with neither a +// kubeconfig loader nor a rest config (NewClient(nil)). +var errNoConfiguration = errors.New("client has no kubeconfig loader and no rest config") -func applyClientOpts(c *Client, opts []ClientOpt) *Client { - if c.isOpenShift == nil { - c.isOpenShift = sync.OnceValues(func() (bool, error) { return detectOpenShift(c) }) - } - for _, o := range opts { - o(c) - } - return c -} +// errNilClient is returned by every method called on a nil *Client. +// +// Go runs a pointer-receiver method on a nil pointer; only a field access +// panics (https://go.dev/ref/spec#Selectors). RestConfig, RawConfig, +// DefaultNamespace and IsOpenShift are the only methods that read the +// struct's fields, and they check the receiver first. Everything else reaches +// the fields through them, so a nil client anywhere yields this error, not a +// panic. +var errNilClient = errors.New("kubernetes client is nil") -// NewClient returns a Client backed by the given client configuration. -func NewClient(cc clientcmd.ClientConfig, opts ...ClientOpt) *Client { - return applyClientOpts(&Client{cc: cc}, opts) +// NewClient returns a Client backed by the given kubeconfig loader. +func NewClient(cc clientcmd.ClientConfig) *Client { + return newClient(&Client{cc: cc}) } // NewClientFromKubeconfig returns a Client which resolves its configuration // the way kubectl does: KUBECONFIG, ~/.kube/config, in-cluster. -func NewClientFromKubeconfig(opts ...ClientOpt) *Client { - return NewClient(GetClientConfig(), opts...) +func NewClientFromKubeconfig() *Client { + return NewClient(kubeconfigClientConfig()) } // NewClientFromConfig returns a Client backed by an already-resolved rest -// config. Used in tests that do not want to go through kubeconfig. -func NewClientFromConfig(cfg *rest.Config, opts ...ClientOpt) *Client { - return applyClientOpts(&Client{cfg: cfg}, opts) +// configuration, for callers which have a host and credentials but no +// kubeconfig. Methods which need a kubeconfig (RawConfig, DefaultNamespace) +// return an error on such a Client. +func NewClientFromConfig(cfg *rest.Config) *Client { + return newClient(&Client{cfg: cfg}) } -// Loader returns the deferred kubeconfig loader, if this Client was built -// from kubeconfig. Nil when built from NewClientFromConfig. -func (c *Client) Loader() clientcmd.ClientConfig { - return c.cc +func newClient(c *Client) *Client { + c.isOpenShift = sync.OnceValues(func() (bool, error) { return detectOpenShift(c) }) + return c } -// RestConfig returns the resolved rest configuration (host, token, certs). +// RestConfig returns the rest configuration (host, token, certs). Every call +// returns a value the caller may mutate: a copy for a config-backed Client, +// and for a kubeconfig-backed one the loader builds a new rest.Config on +// each call. func (c *Client) RestConfig() (*rest.Config, error) { - c.cfgOnce.Do(func() { - if c.cfg != nil { - return - } - if c.cc == nil { - c.cfgErr = fmt.Errorf("no kubernetes client configuration available") - return - } - c.cfg, c.cfgErr = c.cc.ClientConfig() - if c.cfgErr != nil { - c.cfgErr = fmt.Errorf("failed to create kubernetes client config: %w", c.cfgErr) - } - }) - return c.cfg, c.cfgErr + if c == nil { + return nil, errNilClient + } + if c.cfg != nil { + return rest.CopyConfig(c.cfg), nil + } + if c.cc == nil { + return nil, errNoConfiguration + } + cfg, err := c.cc.ClientConfig() + if err != nil { + return nil, fmt.Errorf("failed to create kubernetes client config: %w", err) + } + return cfg, nil } // RawConfig returns the merged kubeconfig. func (c *Client) RawConfig() (clientcmdapi.Config, error) { + if c == nil { + return clientcmdapi.Config{}, errNilClient + } if c.cc == nil { - return clientcmdapi.Config{}, fmt.Errorf("no kubernetes client configuration available") + return clientcmdapi.Config{}, errNoKubeconfig } return c.cc.RawConfig() } @@ -124,8 +137,11 @@ func (c *Client) DynamicClient() (dynamic.Interface, error) { // DefaultNamespace returns the namespace of the active context. func (c *Client) DefaultNamespace() (string, error) { + if c == nil { + return "", errNilClient + } if c.cc == nil { - return "", fmt.Errorf("no kubernetes client configuration available") + return "", errNoKubeconfig } ns, _, err := c.cc.Namespace() return ns, err @@ -148,12 +164,15 @@ func (c *Client) ClientAndNamespace(ns string) (*kubernetes.Clientset, string, e // A non-nil error means the cluster could not be asked and the bool is // meaningless. Detection runs at most once per Client, on first use. func (c *Client) IsOpenShift() (bool, error) { + if c == nil { + return false, errNilClient + } return c.isOpenShift() } // detectOpenShift asks the API server for the route.openshift.io/v1 group, -// which works even under restrictive RBAC. Any error, including an -// unreachable cluster, counts as "not OpenShift". +// which works even under restrictive RBAC. NotFound means the cluster answered +// "not OpenShift"; any other error means it could not be asked. func detectOpenShift(c *Client) (bool, error) { cs, err := c.Clientset() if err != nil { @@ -171,48 +190,9 @@ func detectOpenShift(c *Client) (bool, error) { } } -// The functions below read the kubeconfig ad hoc. They remain only for -// callers not yet migrated to Client and will be removed once none are left. -// Do not add new callers. - -func NewClientAndResolvedNamespace(ns string) (*kubernetes.Clientset, string, error) { - var err error - if ns == "" { - ns, err = GetDefaultNamespace() - if err != nil { - return nil, ns, err - } - } - - client, err := NewKubernetesClientset() - return client, ns, err -} - -func NewKubernetesClientset() (*kubernetes.Clientset, error) { - restConfig, err := GetClientConfig().ClientConfig() - if err != nil { - return nil, fmt.Errorf("failed to create new kubernetes client: %w", err) - } - - return kubernetes.NewForConfig(restConfig) -} - -func NewDynamicClient() (dynamic.Interface, error) { - restConfig, err := GetClientConfig().ClientConfig() - if err != nil { - return nil, fmt.Errorf("failed to create new kubernetes client: %w", err) - } - - return dynamic.NewForConfig(restConfig) -} - -// GetDefaultNamespace returns default namespace -func GetDefaultNamespace() (namespace string, err error) { - namespace, _, err = GetClientConfig().Namespace() - return -} - -func GetClientConfig() clientcmd.ClientConfig { +// kubeconfigClientConfig resolves cluster configuration the way kubectl +// does: KUBECONFIG, ~/.kube/config, in-cluster. +func kubeconfigClientConfig() clientcmd.ClientConfig { return clientcmd.NewNonInteractiveDeferredLoadingClientConfig( clientcmd.NewDefaultClientConfigLoadingRules(), &clientcmd.ConfigOverrides{}) diff --git a/pkg/k8s/client_test.go b/pkg/k8s/client_test.go index d2cbeafb2b..845301019c 100644 --- a/pkg/k8s/client_test.go +++ b/pkg/k8s/client_test.go @@ -12,22 +12,67 @@ import ( . "knative.dev/func/pkg/testing" ) -func TestNewClientFromConfig(t *testing.T) { - cfg := &rest.Config{Host: "https://example.com:6443"} +// inMemoryLoader returns a kubeconfig loader for a single-context config +// pointing at host, without touching the filesystem. +func inMemoryLoader(host string) clientcmd.ClientConfig { + return clientcmd.NewDefaultClientConfig(clientcmdapi.Config{ + CurrentContext: "test", + Contexts: map[string]*clientcmdapi.Context{"test": {Cluster: "test", AuthInfo: "test"}}, + Clusters: map[string]*clientcmdapi.Cluster{"test": {Server: host}}, + AuthInfos: map[string]*clientcmdapi.AuthInfo{"test": {Token: "t"}}, + }, nil) +} + +// TestClient_RestConfig verifies the rest config is resolved from the loader +// and that each call yields its own copy, safe to mutate. +func TestClient_RestConfig(t *testing.T) { + c := k8s.NewClient(inMemoryLoader("https://example.com:6443")) + + got, err := c.RestConfig() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.Host != "https://example.com:6443" { + t.Errorf("unexpected host %q", got.Host) + } + got.Host = "mutated" + if again, _ := c.RestConfig(); again.Host != "https://example.com:6443" { + t.Error("RestConfig() should return a fresh config on every call") + } +} + +// TestClient_FromConfig verifies a Client built from a bare rest.Config: +// RestConfig returns a copy of it, and the kubeconfig-only methods report +// that no kubeconfig is available rather than panicking. +func TestClient_FromConfig(t *testing.T) { + cfg := &rest.Config{Host: "https://example.com:6443", BearerToken: "t"} c := k8s.NewClientFromConfig(cfg) got, err := c.RestConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != cfg { - t.Error("RestConfig() should return the same config that was passed in") + if got.Host != cfg.Host || got.BearerToken != cfg.BearerToken { + t.Errorf("unexpected config %+v", got) + } + got.Host = "mutated" + if cfg.Host != "https://example.com:6443" { + t.Error("RestConfig() must return a copy, not the caller's config") + } + + if _, err := c.Clientset(); err != nil { + t.Errorf("unexpected error creating clientset: %v", err) + } + if _, err := c.RawConfig(); err == nil { + t.Error("RawConfig() should fail without a kubeconfig") + } + if _, err := c.DefaultNamespace(); err == nil { + t.Error("DefaultNamespace() should fail without a kubeconfig") } } func TestClientClientset(t *testing.T) { - cfg := &rest.Config{Host: "https://example.com:6443"} - c := k8s.NewClientFromConfig(cfg) + c := k8s.NewClient(inMemoryLoader("https://example.com:6443")) cs, err := c.Clientset() if err != nil { @@ -103,19 +148,6 @@ func TestClient_IsOpenShiftUnreachable(t *testing.T) { } } -// TestClient_WithOpenShift verifies the preset answer wins and no detection -// runs, even though the fake cluster would answer the opposite. -func TestClient_WithOpenShift(t *testing.T) { - FakeCluster(t, false) - if ok, _ := k8s.NewClientFromKubeconfig(k8s.WithOpenShift(true)).IsOpenShift(); !ok { - t.Error("expected preset true") - } - FakeCluster(t, true) - if ok, _ := k8s.NewClientFromKubeconfig(k8s.WithOpenShift(false)).IsOpenShift(); ok { - t.Error("expected preset false") - } -} - // TestClient_Namespace verifies namespace resolution from the active context. func TestClient_Namespace(t *testing.T) { FakeCluster(t, false) @@ -167,3 +199,44 @@ func TestClient_OpenShiftDockerCredentialLoaders(t *testing.T) { t.Error("expected error for a foreign registry") } } + +// TestNilClient_MethodsReturnErrors ensures every method of a nil *Client +// returns an error instead of panicking (see errNilClient). +func TestNilClient_MethodsReturnErrors(t *testing.T) { + var c *k8s.Client + + if _, err := c.RestConfig(); err == nil { + t.Error("RestConfig() on a nil client should fail") + } + if _, err := c.RawConfig(); err == nil { + t.Error("RawConfig() on a nil client should fail") + } + if _, err := c.DefaultNamespace(); err == nil { + t.Error("DefaultNamespace() on a nil client should fail") + } + if _, err := c.IsOpenShift(); err == nil { + t.Error("IsOpenShift() on a nil client should fail") + } + // Derived methods reach the fields only through the four above. + if _, err := c.Clientset(); err == nil { + t.Error("Clientset() on a nil client should fail") + } + if _, err := c.DynamicClient(); err == nil { + t.Error("DynamicClient() on a nil client should fail") + } + if _, _, err := c.ClientAndNamespace(""); err == nil { + t.Error("ClientAndNamespace() on a nil client should fail") + } + if _, err := c.OpenShiftServiceCA(t.Context()); err == nil { + t.Error("OpenShiftServiceCA() on a nil client should fail") + } + if loaders := c.OpenShiftDockerCredentialLoaders(); loaders != nil { + t.Error("OpenShiftDockerCredentialLoaders() on a nil client should yield no loaders") + } + + // A Client with neither a kubeconfig loader nor a rest config fails the + // same way, one level up. + if _, err := k8s.NewClient(nil).RestConfig(); err == nil { + t.Error("RestConfig() on a Client without configuration should fail") + } +} diff --git a/pkg/k8s/deployer_int_test.go b/pkg/k8s/deployer_int_test.go index cf79b4f128..bf0641305d 100644 --- a/pkg/k8s/deployer_int_test.go +++ b/pkg/k8s/deployer_int_test.go @@ -10,7 +10,7 @@ import ( ) func TestInt_FullPath(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_FullPath(t, k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), k8s.NewRemover(kc, false), @@ -20,7 +20,7 @@ func TestInt_FullPath(t *testing.T) { } func TestInt_Deploy(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_Deploy(t, k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), k8s.NewRemover(kc, false), @@ -29,7 +29,7 @@ func TestInt_Deploy(t *testing.T) { } func TestInt_Metadata(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_Metadata(t, k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), k8s.NewRemover(kc, false), @@ -38,7 +38,7 @@ func TestInt_Metadata(t *testing.T) { } func TestInt_Events(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() t.Skip("Kubernetes deploy does not support func subscribe yet") deployertesting.TestInt_Events(t, @@ -49,7 +49,7 @@ func TestInt_Events(t *testing.T) { } func TestInt_Scale(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_Scale(t, k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), k8s.NewRemover(kc, false), @@ -58,7 +58,7 @@ func TestInt_Scale(t *testing.T) { } func TestInt_EnvsUpdate(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_EnvsUpdate(t, k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), k8s.NewRemover(kc, false), @@ -67,7 +67,7 @@ func TestInt_EnvsUpdate(t *testing.T) { } func TestInt_ResourceValidationOnFirstDeploy(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_ResourceValidationOnFirstDeploy(t, k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), k8s.NewRemover(kc, false), @@ -76,7 +76,7 @@ func TestInt_ResourceValidationOnFirstDeploy(t *testing.T) { } func TestInt_OperatorSync(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_OperatorSync(t, k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), k8s.NewRemover(kc, false), diff --git a/pkg/k8s/describer_int_test.go b/pkg/k8s/describer_int_test.go index 264477ea4e..29eddecc0e 100644 --- a/pkg/k8s/describer_int_test.go +++ b/pkg/k8s/describer_int_test.go @@ -10,7 +10,7 @@ import ( ) func TestInt_Describe(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() describertesting.TestInt_Describe(t, k8s.NewDescriber(kc, true), k8s.NewDeployer(kc, k8s.WithDeployerVerbose(true)), diff --git a/pkg/k8s/dialer.go b/pkg/k8s/dialer.go index af81b77440..73ae9b9c60 100644 --- a/pkg/k8s/dialer.go +++ b/pkg/k8s/dialer.go @@ -255,7 +255,6 @@ func (c *contextDialer) startDialerPod(ctx context.Context) (err error) { if err != nil { return } - c.restConf = restclient.CopyConfig(c.restConf) // the Client's config is shared; do not mutate it c.restConf.WarningHandler = restclient.NoWarnings{} err = setConfigDefaults(c.restConf) diff --git a/pkg/k8s/dialer_int_test.go b/pkg/k8s/dialer_int_test.go index c1a737865c..7adf1b24ee 100644 --- a/pkg/k8s/dialer_int_test.go +++ b/pkg/k8s/dialer_int_test.go @@ -35,10 +35,10 @@ func TestInt_DialInClusterService(t *testing.T) { var ctx = t.Context() // Initialize client configuration from kubeconfig or in-cluster config - clientConfig := k8s.GetClientConfig() + kc := k8s.NewClientFromKubeconfig() // Extract the REST config and create a clientset for API operations - rc, err := clientConfig.ClientConfig() + rc, err := kc.RestConfig() if err != nil { t.Fatal(err) } @@ -56,7 +56,7 @@ func TestInt_DialInClusterService(t *testing.T) { } // Determine which namespace to use for test resources - testingNS, _, err := clientConfig.Namespace() + testingNS, err := kc.DefaultNamespace() if err != nil { t.Fatal(err) } @@ -157,7 +157,7 @@ func TestInt_DialInClusterService(t *testing.T) { // Initialize the InClusterDialer. This will create a socat pod in the // cluster that acts as a TCP proxy, allowing us to reach cluster-internal // services. The "lazy init" variant only creates the pod when first used. - dialer := k8s.NewLazyInitInClusterDialer(k8s.NewClient(clientConfig)) + dialer := k8s.NewLazyInitInClusterDialer(kc) t.Cleanup(func() { dialer.Close() }) @@ -261,8 +261,8 @@ func TestInt_DialUnreachable(t *testing.T) { func TestInt_DialContextExpiry(t *testing.T) { var setupCtx = t.Context() - clientConfig := k8s.GetClientConfig() - rc, err := clientConfig.ClientConfig() + kc := k8s.NewClientFromKubeconfig() + rc, err := kc.RestConfig() if err != nil { t.Fatal(err) } @@ -275,7 +275,7 @@ func TestInt_DialContextExpiry(t *testing.T) { creatOpts := metaV1.CreateOptions{} deleteOpts := metaV1.DeleteOptions{PropagationPolicy: &pp} - testingNS, _, err := clientConfig.Namespace() + testingNS, err := kc.DefaultNamespace() if err != nil { t.Fatal(err) } @@ -326,7 +326,7 @@ func TestInt_DialContextExpiry(t *testing.T) { } // Create the dialer pod eagerly so that pod creation is not tied to dialCtx. - dialer, err := k8s.NewInClusterDialer(setupCtx, k8s.NewClient(clientConfig)) + dialer, err := k8s.NewInClusterDialer(setupCtx, kc) if err != nil { t.Fatal(err) } diff --git a/pkg/k8s/lister_int_test.go b/pkg/k8s/lister_int_test.go index 633692bf84..307cb240d3 100644 --- a/pkg/k8s/lister_int_test.go +++ b/pkg/k8s/lister_int_test.go @@ -10,7 +10,7 @@ import ( ) func TestInt_List(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() listertesting.TestInt_List(t, k8s.NewLister(kc, true), k8s.NewDeployer(kc, k8s.WithDeployerVerbose(true)), diff --git a/pkg/k8s/logs_int_test.go b/pkg/k8s/logs_int_test.go index 9bc94f3133..4c6e7620c7 100644 --- a/pkg/k8s/logs_int_test.go +++ b/pkg/k8s/logs_int_test.go @@ -20,7 +20,7 @@ func TestInt_GetPodLogs(t *testing.T) { var err error ctx, cancel := context.WithTimeout(t.Context(), time.Minute*5) t.Cleanup(cancel) - cliSet, err := k8s.NewKubernetesClientset() + cliSet, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } @@ -97,7 +97,7 @@ out: func TestInt_GetPodLogsBySelectorSnapshot(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), time.Minute*5) t.Cleanup(cancel) - cliSet, err := k8s.NewKubernetesClientset() + cliSet, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } diff --git a/pkg/k8s/openshift.go b/pkg/k8s/openshift.go index f417846485..471acf644b 100644 --- a/pkg/k8s/openshift.go +++ b/pkg/k8s/openshift.go @@ -6,11 +6,9 @@ import ( "encoding/pem" "errors" "strings" - "sync" "time" v1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/util/rand" @@ -99,17 +97,6 @@ func (c *Client) DefaultOpenShiftRegistry() string { return openShiftRegistryHostPort + "/" + ns } -// GetDefaultOpenShiftRegistry reads the kubeconfig ad hoc. Remains only for -// callers not yet migrated to Client. Do not add new callers. -func GetDefaultOpenShiftRegistry() string { - ns, _ := GetDefaultNamespace() - if ns == "" { - ns = "default" - } - - return openShiftRegistryHostPort + "/" + ns -} - // IsOpenShiftInternalRegistry returns true if the given registry string // refers to the OpenShift internal image registry. func IsOpenShiftInternalRegistry(registry string) bool { @@ -146,65 +133,6 @@ func (c *Client) OpenShiftDockerCredentialLoaders() []creds.CredentialsCallback } -// openShiftRouteGroupVersion is the API group whose presence identifies an -// OpenShift cluster. Routes are OpenShift-specific, and discovery should answer -// it even under restrictive RBAC, unlike listing namespaces or services. -const openShiftRouteGroupVersion = "route.openshift.io/v1" - -var ( - detectOnce sync.Once - isOpenShift bool - detectErr error -) - -// DetectOpenShift reads the kubeconfig ad hoc. Remains only for callers not -// yet migrated to Client. Do not add new callers; use (*Client).IsOpenShift. -// -// It reports whether the cluster serves the OpenShift Route API. A non-nil -// error means the cluster could not be asked and the bool is meaningless. -// Probes once per process, answers from cache after. -func DetectOpenShift() (bool, error) { - detectOnce.Do(func() { - client, err := NewKubernetesClientset() - if err != nil { - detectErr = err - return - } - _, err = client.Discovery().ServerResourcesForGroupVersion(openShiftRouteGroupVersion) - switch { - case err == nil: - isOpenShift = true - case apierrors.IsNotFound(err): - // The cluster answered: it does not serve this API. - default: - detectErr = err - } - }) - return isOpenShift, detectErr -} - -// IsOpenShift is a convenient wrapper for getting simple yes/no for openshift -// cluster. The inner function should run in the cmd layer once to resolve the -// detectOnce.Do(), any call after is cached so we dont have to call API all the -// time. -// -// note: gauron99: this might change after restructuring to kubeconfig resolution -// at the start of program instead of adhoc API calls of kube client throughout -// the codebase -func IsOpenShift() bool { - ok, _ := DetectOpenShift() - return ok -} - -// SetOpenShiftForTest seeds the detection cache; err simulates a cluster that -// could not be asked. Returns a cleanup restoring the previous state. -func SetOpenShiftForTest(val bool, err error) func() { - detectOnce.Do(func() {}) // ensure real detection won't run - prevB, prevE := isOpenShift, detectErr - isOpenShift, detectErr = val, err - return func() { isOpenShift, detectErr = prevB, prevE } -} - const ( annotationOpenShiftVcsUri = "app.openshift.io/vcs-uri" annotationOpenShiftVcsRef = "app.openshift.io/vcs-ref" diff --git a/pkg/k8s/persistent_volumes.go b/pkg/k8s/persistent_volumes.go index 248159929a..381a8187d7 100644 --- a/pkg/k8s/persistent_volumes.go +++ b/pkg/k8s/persistent_volumes.go @@ -88,7 +88,6 @@ func runWithVolumeMounted(ctx context.Context, c *Client, podImage string, podCo if err != nil { return fmt.Errorf("cannot get client config: %w", err) } - restConf = restclient.CopyConfig(restConf) // the Client's config is shared; do not mutate it restConf.WarningHandler = restclient.NoWarnings{} err = setConfigDefaults(restConf) diff --git a/pkg/k8s/remover_int_test.go b/pkg/k8s/remover_int_test.go index f1212a40f3..445ca19d62 100644 --- a/pkg/k8s/remover_int_test.go +++ b/pkg/k8s/remover_int_test.go @@ -10,7 +10,7 @@ import ( ) func TestInt_Remove(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() removertesting.TestInt_Remove(t, k8s.NewRemover(kc, true), k8s.NewDeployer(kc, k8s.WithDeployerVerbose(true)), diff --git a/pkg/keda/deployer.go b/pkg/keda/deployer.go index c5b5d55403..303d61f3cd 100644 --- a/pkg/keda/deployer.go +++ b/pkg/keda/deployer.go @@ -119,7 +119,8 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu } // Resolved once per deploy and threaded down - interceptorNS, exposeRefusal := interceptorNamespace(ctx, k8sClientset) + openShift, _ := d.kc.IsOpenShift() + interceptorNS, exposeRefusal := interceptorNamespace(ctx, k8sClientset, openShift) // DNS label checks before we create anything on cluster if err := d.validateExposure(f, exposeRefusal); err != nil { diff --git a/pkg/keda/deployer_int_test.go b/pkg/keda/deployer_int_test.go index 448f4cacca..fde810f405 100644 --- a/pkg/keda/deployer_int_test.go +++ b/pkg/keda/deployer_int_test.go @@ -11,7 +11,7 @@ import ( ) func TestInt_FullPath(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_FullPath(t, keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), keda.NewRemover(kc, false), @@ -21,7 +21,7 @@ func TestInt_FullPath(t *testing.T) { } func TestInt_Deploy(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_Deploy(t, keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), keda.NewRemover(kc, false), @@ -30,7 +30,7 @@ func TestInt_Deploy(t *testing.T) { } func TestInt_Metadata(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_Metadata(t, keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), keda.NewRemover(kc, false), @@ -39,7 +39,7 @@ func TestInt_Metadata(t *testing.T) { } func TestInt_Events(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() t.Skip("Keda deployer does not support func subscribe yet") deployertesting.TestInt_Events(t, @@ -50,7 +50,7 @@ func TestInt_Events(t *testing.T) { } func TestInt_Scale(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_Scale(t, keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), keda.NewRemover(kc, false), @@ -59,7 +59,7 @@ func TestInt_Scale(t *testing.T) { } func TestInt_EnvsUpdate(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_EnvsUpdate(t, keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), keda.NewRemover(kc, false), @@ -68,7 +68,7 @@ func TestInt_EnvsUpdate(t *testing.T) { } func TestInt_ResourceValidationOnFirstDeploy(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_ResourceValidationOnFirstDeploy(t, keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), keda.NewRemover(kc, false), @@ -77,7 +77,7 @@ func TestInt_ResourceValidationOnFirstDeploy(t *testing.T) { } func TestInt_OperatorSync(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_OperatorSync(t, keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), keda.NewRemover(kc, false), diff --git a/pkg/keda/describer_int_test.go b/pkg/keda/describer_int_test.go index 60a2271268..2a73a89318 100644 --- a/pkg/keda/describer_int_test.go +++ b/pkg/keda/describer_int_test.go @@ -11,7 +11,7 @@ import ( ) func TestInt_Describe(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() describertesting.TestInt_Describe(t, keda.NewDescriber(kc, true), keda.NewDeployer(kc, keda.WithDeployerVerbose(true)), diff --git a/pkg/keda/exposure.go b/pkg/keda/exposure.go index d3a496e967..0ac0c9b63a 100644 --- a/pkg/keda/exposure.go +++ b/pkg/keda/exposure.go @@ -12,7 +12,6 @@ import ( "knative.dev/func/pkg/deployer" fn "knative.dev/func/pkg/functions" - "knative.dev/func/pkg/k8s" ) const ( @@ -48,8 +47,8 @@ const ( // Off OpenShift only keda is possible, so only keda is probed. Order matters // when every probe is denied: Forbidden says nothing about existence, so the // first candidate becomes the guess. -func interceptorNamespaceCandidates() []string { - if k8s.IsOpenShift() { +func interceptorNamespaceCandidates(openShift bool) []string { + if openShift { return []string{interceptorNamespaceOpenShift, interceptorNamespaceUpstream} } return []string{interceptorNamespaceUpstream} @@ -61,9 +60,9 @@ func interceptorNamespaceCandidates() []string { // for the cluster-local bridge, and exposeRefusal says why it is not good // enough to build a Route to. func interceptorNamespace(ctx context.Context, - clientset kubernetes.Interface) (ns string, exposeRefusal error) { + clientset kubernetes.Interface, openShift bool) (ns string, exposeRefusal error) { - candidates := interceptorNamespaceCandidates() + candidates := interceptorNamespaceCandidates(openShift) var undetermined []string for _, candidate := range candidates { diff --git a/pkg/keda/exposure_test.go b/pkg/keda/exposure_test.go index 06ffb003d6..7131879b79 100644 --- a/pkg/keda/exposure_test.go +++ b/pkg/keda/exposure_test.go @@ -16,7 +16,6 @@ import ( "knative.dev/func/pkg/deployer" fn "knative.dev/func/pkg/functions" - "knative.dev/func/pkg/k8s" ) // errDenied fills the cause slot of the synthetic Forbidden errors the test @@ -31,9 +30,7 @@ var errDenied = errors.New("denied") // // It is resolved by looking for the interceptor Service, so the platform sets // only the order tried and the answer given when nothing definite came back. -// -// Note: SetOpenShiftForTest mutates a package-level bool without a mutex, so -// this test must not run with t.Parallel() (see pkg/k8s/openshift.go). +// The platform is a plain argument, so no global state is involved. func Test_interceptorNamespace(t *testing.T) { interceptorServiceIn := func(ns string) *corev1.Service { return &corev1.Service{ObjectMeta: metav1.ObjectMeta{ @@ -91,16 +88,13 @@ func Test_interceptorNamespace(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cleanup := k8s.SetOpenShiftForTest(tt.openShift, nil) - defer cleanup() - // A fake cluster seeded with whatever interceptor Services this // row installs. objects := make([]runtime.Object, 0, len(tt.installed)) for _, ns := range tt.installed { objects = append(objects, interceptorServiceIn(ns)) } - got, refusal := interceptorNamespace(t.Context(), fake.NewClientset(objects...)) + got, refusal := interceptorNamespace(t.Context(), fake.NewClientset(objects...), tt.openShift) if got != tt.want { t.Errorf("interceptorNamespace() = %q, want %q", got, tt.want) } @@ -125,8 +119,6 @@ func Test_interceptorNamespace(t *testing.T) { // candidate is denied. Test_interceptorNamespace_DeniedBeatsRuledOut is the // case that separates them; this one only holds the floor. func Test_interceptorNamespace_AllDeniedUsesPlatformDefault(t *testing.T) { - cleanup := k8s.SetOpenShiftForTest(true, nil) - defer cleanup() // An interceptor really is installed; denial hides it, so the guess // below is genuinely blind. @@ -138,7 +130,7 @@ func Test_interceptorNamespace_AllDeniedUsesPlatformDefault(t *testing.T) { schema.GroupResource{Resource: "services"}, interceptorServiceName, errDenied) }) - got, refusal := interceptorNamespace(t.Context(), clientset) + got, refusal := interceptorNamespace(t.Context(), clientset, true) if got != interceptorNamespaceOpenShift { t.Errorf("interceptorNamespace() = %q, want the platform default %q when every lookup is denied", got, interceptorNamespaceOpenShift) @@ -155,8 +147,6 @@ func Test_interceptorNamespace_AllDeniedUsesPlatformDefault(t *testing.T) { // ruled out wins. Answering with a namespace known to hold nothing would be // strictly worse than admitting ignorance. func Test_interceptorNamespace_DeniedBeatsRuledOut(t *testing.T) { - cleanup := k8s.SetOpenShiftForTest(true, nil) - defer cleanup() clientset := fake.NewClientset() clientset.PrependReactor("get", "services", func(action k8stesting.Action) (bool, runtime.Object, error) { @@ -167,7 +157,7 @@ func Test_interceptorNamespace_DeniedBeatsRuledOut(t *testing.T) { return false, nil, nil // openshift-keda falls through to a real NotFound }) - got, refusal := interceptorNamespace(t.Context(), clientset) + got, refusal := interceptorNamespace(t.Context(), clientset, true) if got != interceptorNamespaceUpstream { t.Errorf("interceptorNamespace() = %q, want %q: the ruled-out candidate must lose to the unseen one", got, interceptorNamespaceUpstream) diff --git a/pkg/keda/lister_int_test.go b/pkg/keda/lister_int_test.go index 955da22e8a..916902af21 100644 --- a/pkg/keda/lister_int_test.go +++ b/pkg/keda/lister_int_test.go @@ -11,7 +11,7 @@ import ( ) func TestInt_List(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() listertesting.TestInt_List(t, keda.NewLister(kc, true), keda.NewDeployer(kc, keda.WithDeployerVerbose(true)), diff --git a/pkg/keda/remover.go b/pkg/keda/remover.go index 3bb3fda2de..4bb61a8ec4 100644 --- a/pkg/keda/remover.go +++ b/pkg/keda/remover.go @@ -62,7 +62,7 @@ func (remover *Remover) Remove(ctx context.Context, name, ns string) error { // A Route left unrecorded by a crash is not searched for; the next // exposed redeploy finds it by its function labels. if recordedNS := svc.Annotations[k8s.RouteNamespaceAnnotation]; recordedNS != "" { - dynClient, err := k8s.NewDynamicClient() + dynClient, err := remover.kc.DynamicClient() if err != nil { return fmt.Errorf("could not setup dynamic client: %w", err) } diff --git a/pkg/keda/remover_int_test.go b/pkg/keda/remover_int_test.go index 8d406754c2..b46bfed4f7 100644 --- a/pkg/keda/remover_int_test.go +++ b/pkg/keda/remover_int_test.go @@ -11,7 +11,7 @@ import ( ) func TestInt_Remove(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() removertesting.TestInt_Remove(t, keda.NewRemover(kc, true), keda.NewDeployer(kc, keda.WithDeployerVerbose(true)), diff --git a/pkg/knative/deployer_int_test.go b/pkg/knative/deployer_int_test.go index 9b8ea38d2f..986041dd74 100644 --- a/pkg/knative/deployer_int_test.go +++ b/pkg/knative/deployer_int_test.go @@ -11,7 +11,7 @@ import ( ) func TestInt_FullPath(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_FullPath(t, knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), knative.NewRemover(kc, true), @@ -21,7 +21,7 @@ func TestInt_FullPath(t *testing.T) { } func TestInt_Deploy(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_Deploy(t, knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), knative.NewRemover(kc, false), @@ -30,7 +30,7 @@ func TestInt_Deploy(t *testing.T) { } func TestInt_Metadata(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_Metadata(t, knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), knative.NewRemover(kc, false), @@ -39,7 +39,7 @@ func TestInt_Metadata(t *testing.T) { } func TestInt_Events(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_Events(t, knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), knative.NewRemover(kc, false), @@ -48,7 +48,7 @@ func TestInt_Events(t *testing.T) { } func TestInt_Scale(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_Scale(t, knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), knative.NewRemover(kc, false), @@ -57,7 +57,7 @@ func TestInt_Scale(t *testing.T) { } func TestInt_EnvsUpdate(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_EnvsUpdate(t, knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), knative.NewRemover(kc, false), @@ -66,7 +66,7 @@ func TestInt_EnvsUpdate(t *testing.T) { } func TestInt_ResourceValidationOnFirstDeploy(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_ResourceValidationOnFirstDeploy(t, knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), knative.NewRemover(kc, false), @@ -75,7 +75,7 @@ func TestInt_ResourceValidationOnFirstDeploy(t *testing.T) { } func TestInt_OperatorSync(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() deployertesting.TestInt_OperatorSync(t, knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), knative.NewRemover(kc, false), diff --git a/pkg/knative/describer_int_test.go b/pkg/knative/describer_int_test.go index b0484b310b..2b36e57701 100644 --- a/pkg/knative/describer_int_test.go +++ b/pkg/knative/describer_int_test.go @@ -11,7 +11,7 @@ import ( ) func TestInt_Describe(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() describertesting.TestInt_Describe(t, knative.NewDescriber(kc, true), knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), diff --git a/pkg/knative/lister_int_test.go b/pkg/knative/lister_int_test.go index 9078fda08c..43af901667 100644 --- a/pkg/knative/lister_int_test.go +++ b/pkg/knative/lister_int_test.go @@ -11,7 +11,7 @@ import ( ) func TestInt_List(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() listertesting.TestInt_List(t, knative.NewLister(kc, true), knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), diff --git a/pkg/knative/remover_int_test.go b/pkg/knative/remover_int_test.go index d03ede4845..6b7a842759 100644 --- a/pkg/knative/remover_int_test.go +++ b/pkg/knative/remover_int_test.go @@ -11,7 +11,7 @@ import ( ) func TestInt_Remove(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() removertesting.TestInt_Remove(t, knative.NewRemover(kc, true), knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 7031cd1d05..2855d2100a 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -43,10 +43,7 @@ var ensureRegistrySecret = k8s.EnsureDockerRegistrySecretExist // SyncFunctionCR creates or updates a Function CR for the given function. // It sets up Kubernetes clients, checks if the Function CRD exists on the // cluster, and creates or updates the CR accordingly. -func SyncFunctionCR(ctx context.Context, cfg SyncConfig) error { - // TODO: inject the k8s.Client through NewSyncer instead of resolving it - // from the kubeconfig here. - kc := k8s.NewClientFromKubeconfig() +func SyncFunctionCR(ctx context.Context, kc *k8s.Client, cfg SyncConfig) error { restCfg, err := kc.RestConfig() if err != nil { return fmt.Errorf("getting kubernetes config: %w", err) diff --git a/pkg/operator/syncer.go b/pkg/operator/syncer.go index a54af32297..e8899612d6 100644 --- a/pkg/operator/syncer.go +++ b/pkg/operator/syncer.go @@ -2,22 +2,26 @@ package operator import ( "context" + "fmt" fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/docker" funcgit "knative.dev/func/pkg/git" + "knative.dev/func/pkg/k8s" "knative.dev/func/pkg/oci" ) type SyncerOpt func(*Syncer) type Syncer struct { + kc *k8s.Client credentialsProvider oci.CredentialsProvider } -func NewSyncer(opts ...SyncerOpt) *Syncer { - s := &Syncer{} +// NewSyncer returns a Syncer which talks to the cluster kc points at. +func NewSyncer(kc *k8s.Client, opts ...SyncerOpt) *Syncer { + s := &Syncer{kc: kc} for _, o := range opts { o(s) } @@ -31,6 +35,9 @@ func WithCredentialsProvider(cp oci.CredentialsProvider) SyncerOpt { } func (s *Syncer) Sync(ctx context.Context, f fn.Function) error { + if s.kc == nil { + return fmt.Errorf("kubernetes client is not initialized") + } repoURL := f.Build.Git.URL repoBranch := f.Build.Git.Revision repoPath := f.Build.Git.ContextDir @@ -71,7 +78,7 @@ func (s *Syncer) Sync(ctx context.Context, f fn.Function) error { } } - return SyncFunctionCR(ctx, SyncConfig{ + return SyncFunctionCR(ctx, s.kc, SyncConfig{ FunctionName: f.Name, Namespace: namespace, RepoURL: repoURL, diff --git a/pkg/pipelines/tekton/client.go b/pkg/pipelines/tekton/client.go index 399ddc35f2..3dbdd94f99 100644 --- a/pkg/pipelines/tekton/client.go +++ b/pkg/pipelines/tekton/client.go @@ -13,9 +13,9 @@ const ( DefaultWaitingTimeout = 120 * time.Second ) -// NewTektonClient returns TektonV1beta1Client for namespace -func NewTektonClient(namespace string) (*v1.TektonV1Client, error) { - restConfig, err := k8s.GetClientConfig().ClientConfig() +// NewTektonClient returns a TektonV1Client for the cluster kc points at. +func NewTektonClient(kc *k8s.Client) (*v1.TektonV1Client, error) { + restConfig, err := kc.RestConfig() if err != nil { return nil, fmt.Errorf("failed to create new tekton client: %w", err) } @@ -28,8 +28,8 @@ func NewTektonClient(namespace string) (*v1.TektonV1Client, error) { return client, nil } -func NewTektonClients() (*cli.Clients, error) { - restConfig, err := k8s.GetClientConfig().ClientConfig() +func NewTektonClients(kc *k8s.Client) (*cli.Clients, error) { + restConfig, err := kc.RestConfig() if err != nil { return nil, fmt.Errorf("failed to create new tekton clientset: %v", err) } diff --git a/pkg/pipelines/tekton/gitlab_int_test.go b/pkg/pipelines/tekton/gitlab_int_test.go index b13e56ddaa..c09e61e71f 100644 --- a/pkg/pipelines/tekton/gitlab_int_test.go +++ b/pkg/pipelines/tekton/gitlab_int_test.go @@ -115,7 +115,7 @@ func TestInt_Gitlab(t *testing.T) { Password: "", }, nil } - pp := tekton.NewPipelinesProvider( + pp := tekton.NewPipelinesProvider(k8s.NewClientFromKubeconfig(), tekton.WithCredentialsProvider(credentialsProvider), tekton.WithPacURLCallback(func() (string, error) { return "http://" + pacCtrHostname, nil @@ -613,7 +613,7 @@ func generateSSHKeys(t *testing.T) string { func usingNamespace(t *testing.T) string { name := "gitlab-test-" + strings.ToLower(random.AlphaString(5)) - k8sClient, err := k8s.NewKubernetesClientset() + k8sClient, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } @@ -667,7 +667,7 @@ func usingNamespace(t *testing.T) string { func awaitBuildCompletion(t *testing.T, name, ns string) <-chan error { - clis, err := tekton.NewTektonClients() + clis, err := tekton.NewTektonClients(k8s.NewClientFromKubeconfig()) if err != nil { t.Fatal(err) } diff --git a/pkg/pipelines/tekton/pac/client.go b/pkg/pipelines/tekton/pac/client.go index 64687d8c17..0047999812 100644 --- a/pkg/pipelines/tekton/pac/client.go +++ b/pkg/pipelines/tekton/pac/client.go @@ -9,16 +9,16 @@ import ( ) // NewTektonPacClientAndResolvedNamespace returns PipelinesascodeV1alpha1Client,namespace,error -func NewTektonPacClientAndResolvedNamespace(namespace string) (*pacv1alpha1.PipelinesascodeV1alpha1Client, string, error) { +func NewTektonPacClientAndResolvedNamespace(kc *k8s.Client, namespace string) (*pacv1alpha1.PipelinesascodeV1alpha1Client, string, error) { var err error if namespace == "" { - namespace, err = k8s.GetDefaultNamespace() + namespace, err = kc.DefaultNamespace() if err != nil { return nil, "", err } } - restConfig, err := k8s.GetClientConfig().ClientConfig() + restConfig, err := kc.RestConfig() if err != nil { return nil, namespace, fmt.Errorf("failed to create new tekton pac client: %w", err) } diff --git a/pkg/pipelines/tekton/pac/pac.go b/pkg/pipelines/tekton/pac/pac.go index 97d7b06a10..c95789a6ac 100644 --- a/pkg/pipelines/tekton/pac/pac.go +++ b/pkg/pipelines/tekton/pac/pac.go @@ -23,15 +23,15 @@ const ( // DetectPACInstallation checks whether PAC is installed on the cluster // Taken and slightly modified from https://github.com/openshift-pipelines/pipelines-as-code/blob/6a7f043f9bb51d04ab729505b26446695595df1f/pkg/cmd/tknpac/bootstrap/bootstrap.go -func DetectPACInstallation(ctx context.Context) (bool, string, error) { +func DetectPACInstallation(ctx context.Context, kc *k8s.Client) (bool, string, error) { var installed bool - clientPac, cns, err := NewTektonPacClientAndResolvedNamespace("") + clientPac, cns, err := NewTektonPacClientAndResolvedNamespace(kc, "") if err != nil { return false, "", err } - clientK8s, _, err := k8s.NewClientAndResolvedNamespace("") + clientK8s, err := kc.Clientset() if err != nil { return false, "", err } @@ -68,12 +68,12 @@ func DetectPACInstallation(ctx context.Context) (bool, string, error) { // DetectPACOpenShiftRoute detect the openshift route where the pac controller is running // Taken and slightly modified from https://github.com/openshift-pipelines/pipelines-as-code/blob/0d63e6239f4a7f1fc90decde1e0a154ed56ed0e7/pkg/cmd/tknpac/bootstrap/route.go -func DetectPACOpenShiftRoute(ctx context.Context, targetNamespace string) (string, error) { +func DetectPACOpenShiftRoute(ctx context.Context, kc *k8s.Client, targetNamespace string) (string, error) { gvr := schema.GroupVersionResource{ Group: openShiftRouteGroup, Version: openShiftRouteVersion, Resource: openShiftRouteResource, } - client, err := k8s.NewDynamicClient() + client, err := kc.DynamicClient() if err != nil { return "", err } @@ -105,8 +105,8 @@ func DetectPACOpenShiftRoute(ctx context.Context, targetNamespace string) (strin // GetPACInfo returns the controller url that PAC controller is running // Taken and slightly modified from https://github.com/openshift-pipelines/pipelines-as-code/blob/0d63e6239f4a7f1fc90decde1e0a154ed56ed0e7/pkg/cli/info/configmap.go -func GetPACInfo(ctx context.Context, namespace string) (string, error) { - client, namespace, err := k8s.NewClientAndResolvedNamespace(namespace) +func GetPACInfo(ctx context.Context, kc *k8s.Client, namespace string) (string, error) { + client, namespace, err := kc.ClientAndNamespace(namespace) if err != nil { return "", err } diff --git a/pkg/pipelines/tekton/pipelines_int_test.go b/pkg/pipelines/tekton/pipelines_int_test.go index 1a0a60da01..4a041e6991 100644 --- a/pkg/pipelines/tekton/pipelines_int_test.go +++ b/pkg/pipelines/tekton/pipelines_int_test.go @@ -45,14 +45,14 @@ const ( ) func newRemoteTestClient(verbose bool, deployer string, opts ...fn.Option) *fn.Client { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() baseOpts := []fn.Option{ fn.WithBuilder(buildpacks.NewBuilder(buildpacks.WithVerbose(verbose))), fn.WithPusher(docker.NewPusher(docker.WithCredentialsProvider(testCP))), fn.WithDescribers(knative.NewDescriber(kc, verbose), k8s.NewDescriber(kc, verbose), keda.NewDescriber(kc, verbose)), fn.WithListers(knative.NewLister(kc, verbose), k8s.NewLister(kc, verbose), keda.NewLister(kc, verbose)), fn.WithRemovers(knative.NewRemover(kc, verbose), k8s.NewRemover(kc, verbose), keda.NewRemover(kc, verbose)), - fn.WithPipelinesProvider(tekton.NewPipelinesProvider(tekton.WithCredentialsProvider(testCP), tekton.WithVerbose(verbose), tekton.WithK8sClient(kc))), + fn.WithPipelinesProvider(tekton.NewPipelinesProvider(kc, tekton.WithCredentialsProvider(testCP), tekton.WithVerbose(verbose))), } switch deployer { @@ -195,7 +195,7 @@ func TestInt_Remote_Default(t *testing.T) { func setupNS(t *testing.T) string { name := "pipeline-integration-test-" + strings.ToLower(random.AlphaString(5)) - cliSet, err := k8s.NewKubernetesClientset() + cliSet, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) } diff --git a/pkg/pipelines/tekton/pipelines_pac_provider.go b/pkg/pipelines/tekton/pipelines_pac_provider.go index d04bfc527d..5a6a9d5930 100644 --- a/pkg/pipelines/tekton/pipelines_pac_provider.go +++ b/pkg/pipelines/tekton/pipelines_pac_provider.go @@ -25,6 +25,9 @@ import ( // Parameter `metadata` is type `any` to not bring `pkg/pipelines` package dependency to `pkg/functions`, // this specific implementation expects the parameter to be a type `pipelines.PacMetada`. func (pp *PipelinesProvider) ConfigurePAC(ctx context.Context, f fn.Function, metadata any) error { + if pp.kc == nil { + return fmt.Errorf("kubernetes client is not initialized") + } // Derive Registry from the image when not explicitly set, so that // downstream insecure-registry detection and template params work. if f.Registry == "" { @@ -95,6 +98,9 @@ func (pp *PipelinesProvider) ConfigurePAC(ctx context.Context, f fn.Function, me // RemovePAC tries to remove all local and remote resources that were created for PAC. // Resources on the remote GitHub repo are not removed, we would need to store webhook id somewhere locally. func (pp *PipelinesProvider) RemovePAC(ctx context.Context, f fn.Function, metadata any) error { + if pp.kc == nil { + return fmt.Errorf("kubernetes client is not initialized") + } data, ok := metadata.(pipelines.PacMetadata) if !ok { return fmt.Errorf("incorrect type of pipelines metadata: %T", metadata) @@ -157,7 +163,7 @@ func (pp *PipelinesProvider) createClusterPACResources(ctx context.Context, f fn } // figure out pac installation namespace - installed, _, err := pac.DetectPACInstallation(ctx) + installed, _, err := pac.DetectPACInstallation(ctx, pp.kc) if !installed { errMsg := "" if err != nil { @@ -213,7 +219,7 @@ func (pp *PipelinesProvider) createClusterPACResources(ctx context.Context, f fn } fmt.Printf(" ✅ Credentials are present on the cluster in secret %q\n", getPipelineSecretName(f)) - err = ensurePACRepositoryExists(ctx, f, namespace, metadata, labels) + err = ensurePACRepositoryExists(ctx, pp.kc, f, namespace, metadata, labels) if err != nil { return err } @@ -228,7 +234,7 @@ func (pp *PipelinesProvider) createClusterPACResources(ctx context.Context, f fn func (pp *PipelinesProvider) createRemotePACResources(ctx context.Context, f fn.Function, metadata pipelines.PacMetadata) error { // figure out pac installation namespace - installed, installationNS, err := pac.DetectPACInstallation(ctx) + installed, installationNS, err := pac.DetectPACInstallation(ctx, pp.kc) if !installed { errMsg := "" if err != nil { @@ -241,14 +247,14 @@ func (pp *PipelinesProvider) createRemotePACResources(ctx context.Context, f fn. } // fetch configmap to get controller url - controllerURL, err := pac.GetPACInfo(ctx, installationNS) + controllerURL, err := pac.GetPACInfo(ctx, pp.kc, installationNS) if err != nil { return err } // check if info configmap has url then use that otherwise try to detect if controllerURL == "" { - controllerURL, _ = pac.DetectPACOpenShiftRoute(ctx, installationNS) + controllerURL, _ = pac.DetectPACOpenShiftRoute(ctx, pp.kc, installationNS) } // we haven't been able to detect PAC controller public route, let's prompt: diff --git a/pkg/pipelines/tekton/pipelines_pac_provider_test.go b/pkg/pipelines/tekton/pipelines_pac_provider_test.go index 8fe536f6a1..76da6236f2 100644 --- a/pkg/pipelines/tekton/pipelines_pac_provider_test.go +++ b/pkg/pipelines/tekton/pipelines_pac_provider_test.go @@ -6,6 +6,7 @@ import ( "knative.dev/func/pkg/builders" fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" . "knative.dev/func/pkg/testing" ) @@ -50,7 +51,8 @@ func Test_createLocalResources(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - pp := NewPipelinesProvider() + // Local resources only; the client is never used. + pp := NewPipelinesProvider(k8s.NewClientFromKubeconfig()) err = pp.createLocalPACResources(t.Context(), f) if (err != nil) != tt.wantErr { t.Errorf("pp.createLocalResources() error = %v, wantErr %v", err, tt.wantErr) @@ -74,7 +76,8 @@ func Test_deleteAllPipelineTemplates(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - pp := NewPipelinesProvider() + // Local resources only; the client is never used. + pp := NewPipelinesProvider(k8s.NewClientFromKubeconfig()) err = pp.createLocalPACResources(t.Context(), f) if err != nil { t.Errorf("unexpected error while running pp.createLocalResources() error = %v", err) diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index f2f2dc8a58..82affe6652 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -85,20 +85,17 @@ func WithTransport(transport http.RoundTripper) Opt { } } -func WithK8sClient(kc *k8s.Client) Opt { - return func(pp *PipelinesProvider) { - pp.kc = kc - } -} - func WithPacURLCallback(getPacURL pacURLCallback) Opt { return func(pp *PipelinesProvider) { pp.getPacURL = getPacURL } } -func NewPipelinesProvider(opts ...Opt) *PipelinesProvider { +// NewPipelinesProvider returns a provider which talks to the cluster kc +// points at. +func NewPipelinesProvider(kc *k8s.Client, opts ...Opt) *PipelinesProvider { pp := &PipelinesProvider{ + kc: kc, getPacURL: func() (string, error) { var url string e := survey.AskOne(&survey.Input{ @@ -123,6 +120,9 @@ func NewPipelinesProvider(opts ...Opt) *PipelinesProvider { // (f.Deploy.Image, f.Deploy.Namespace, f.Deploy.Deployer and f.Deploy.Expose) // or an error. func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn.Function, error) { + if pp.kc == nil { + return "", f, fmt.Errorf("kubernetes client is not initialized") + } var err error // Checks builder and registry: @@ -174,7 +174,7 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn // deployer wrote at exposure time. // Client for the given namespace - client, err := NewTektonClient(namespace) + client, err := NewTektonClient(pp.kc) if err != nil { return "", f, err } @@ -426,6 +426,9 @@ func sourcesAsTarStream(f fn.Function) *io.PipeReader { // Remove tries to remove all resources that are present on the cluster and belongs to the input function and it's pipelines func (pp *PipelinesProvider) Remove(ctx context.Context, f fn.Function) error { + if pp.kc == nil { + return fmt.Errorf("kubernetes client is not initialized") + } return pp.removeClusterResources(ctx, f) } @@ -448,15 +451,11 @@ func (pp *PipelinesProvider) removeClusterResources(ctx context.Context, f fn.Fu // let's try to delete all resources in parallel, so the operation doesn't take long wg := sync.WaitGroup{} - deleteFunctions := []func(context.Context, string, metav1.ListOptions) error{ + deleteFunctions := []func(context.Context, *k8s.Client, string, metav1.ListOptions) error{ deletePipelines, deletePipelineRuns, - func(ctx context.Context, ns string, o metav1.ListOptions) error { - return k8s.DeleteSecrets(ctx, pp.kc, ns, o) - }, - func(ctx context.Context, ns string, o metav1.ListOptions) error { - return k8s.DeletePersistentVolumeClaims(ctx, pp.kc, ns, o) - }, + k8s.DeleteSecrets, + k8s.DeletePersistentVolumeClaims, deletePACRepositories, } @@ -467,7 +466,7 @@ func (pp *PipelinesProvider) removeClusterResources(ctx context.Context, f fn.Fu df := deleteFunctions[i] go func() { defer wg.Done() - err := df(ctx, namespace, listOptions) + err := df(ctx, pp.kc, namespace, listOptions) if err != nil && !k8serrors.IsNotFound(err) && !k8serrors.IsForbidden(err) { errChan <- err } @@ -501,7 +500,7 @@ func (pp *PipelinesProvider) watchPipelineRunProgress(ctx context.Context, pr *v "deploy": "Deploying function to the cluster", } - clients, err := NewTektonClients() + clients, err := NewTektonClients(pp.kc) if err != nil { return err } diff --git a/pkg/pipelines/tekton/resources.go b/pkg/pipelines/tekton/resources.go index be9c36b10a..1c628d2eae 100644 --- a/pkg/pipelines/tekton/resources.go +++ b/pkg/pipelines/tekton/resources.go @@ -12,14 +12,15 @@ import ( "knative.dev/func/pkg/builders" "knative.dev/func/pkg/buildpacks" fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" "knative.dev/func/pkg/s2i" ) -func deletePipelines(ctx context.Context, namespace string, listOptions metav1.ListOptions) (err error) { +func deletePipelines(ctx context.Context, kc *k8s.Client, namespace string, listOptions metav1.ListOptions) (err error) { if namespace == "" { return errors.New("delete pipeline: namespace required") } - client, err := NewTektonClient(namespace) + client, err := NewTektonClient(kc) if err != nil { return } @@ -27,11 +28,11 @@ func deletePipelines(ctx context.Context, namespace string, listOptions metav1.L return client.Pipelines(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, listOptions) } -func deletePipelineRuns(ctx context.Context, namespace string, listOptions metav1.ListOptions) (err error) { +func deletePipelineRuns(ctx context.Context, kc *k8s.Client, namespace string, listOptions metav1.ListOptions) (err error) { if namespace == "" { return errors.New("delete pipeline run: namespace required") } - client, err := NewTektonClient(namespace) + client, err := NewTektonClient(kc) if err != nil { return } diff --git a/pkg/pipelines/tekton/resources_pac.go b/pkg/pipelines/tekton/resources_pac.go index 1398cd3254..8cce17953b 100644 --- a/pkg/pipelines/tekton/resources_pac.go +++ b/pkg/pipelines/tekton/resources_pac.go @@ -41,8 +41,8 @@ func ensurePACSecretExists(ctx context.Context, kc *k8s.Client, f fn.Function, n } // ensurePACRepositoryExists checks that up-to-date Repository CR is present on the cluster -func ensurePACRepositoryExists(ctx context.Context, f fn.Function, namespace string, metadata pipelines.PacMetadata, labels map[string]string) error { - client, namespace, err := pac.NewTektonPacClientAndResolvedNamespace(namespace) +func ensurePACRepositoryExists(ctx context.Context, kc *k8s.Client, f fn.Function, namespace string, metadata pipelines.PacMetadata, labels map[string]string) error { + client, namespace, err := pac.NewTektonPacClientAndResolvedNamespace(kc, namespace) if err != nil { return err } @@ -100,8 +100,8 @@ func ensurePACRepositoryExists(ctx context.Context, f fn.Function, namespace str } // deletePACRepositories deletes all Repository resources present on the cluster that match input list options -func deletePACRepositories(ctx context.Context, namespaceOverride string, listOptions metav1.ListOptions) error { - client, namespace, err := pac.NewTektonPacClientAndResolvedNamespace(namespaceOverride) +func deletePACRepositories(ctx context.Context, kc *k8s.Client, namespaceOverride string, listOptions metav1.ListOptions) error { + client, namespace, err := pac.NewTektonPacClientAndResolvedNamespace(kc, namespaceOverride) if err != nil { return err } diff --git a/pkg/testing/k8s/testing.go b/pkg/testing/k8s/testing.go index 954c564fa1..d3ed778a44 100644 --- a/pkg/testing/k8s/testing.go +++ b/pkg/testing/k8s/testing.go @@ -17,7 +17,7 @@ const DefaultIntTestNamespacePrefix = "func-int-test" func Namespace(t *testing.T, ctx context.Context) string { t.Helper() - cliSet, err := k8s.NewKubernetesClientset() + cliSet, err := k8s.NewClientFromKubeconfig().Clientset() if err != nil { t.Fatal(err) }