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..ff491ad007 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), @@ -88,7 +93,7 @@ func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) { docker.WithTransport(t), docker.WithVerbose(cfg.Verbose), docker.WithInsecure(cfg.InsecureSkipVerify))), - fn.WithSyncer(operator.NewSyncer(operator.WithCredentialsProvider(c))), + fn.WithSyncer(operator.NewSyncer(kc, operator.WithCredentialsProvider(c))), } ) @@ -107,10 +112,19 @@ func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) { return client, cleanup } +// newK8sClient returns kc, or a client resolved from the kubeconfig when kc +// is nil. This is the one place a command falls back to the kubeconfig. +func newK8sClient(kc *k8s.Client) *k8s.Client { + if kc != nil { + return kc + } + return k8s.NewClientFromKubeconfig() +} + // newTransport returns a transport with cluster-flavor-specific variations // which take advantage of additional features offered by cluster variants. -func newTransport(insecureSkipVerify bool) fnhttp.RoundTripCloser { - return fnhttp.NewRoundTripper(fnhttp.WithInsecureSkipVerify(insecureSkipVerify), fnhttp.WithOpenShiftServiceCA()) +func newTransport(kc *k8s.Client, insecureSkipVerify bool) fnhttp.RoundTripCloser { + return fnhttp.NewRoundTripper(kc, fnhttp.WithInsecureSkipVerify(insecureSkipVerify), fnhttp.WithOpenShiftServiceCA(kc)) } // newCredentialsProvider returns a credentials provider which possibly @@ -118,8 +132,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 +170,22 @@ func newCredentialsProvider(configPath string, t http.RoundTripper, authFilePath return creds.NewCredentialsProvider(configPath, options...) } -func newTektonPipelinesProvider(creds oci.CredentialsProvider, verbose bool, transport http.RoundTripper) *tekton.PipelinesProvider { +func newTektonPipelinesProvider(kc *k8s.Client, creds oci.CredentialsProvider, verbose bool, transport http.RoundTripper) *tekton.PipelinesProvider { options := []tekton.Opt{ tekton.WithCredentialsProvider(creds), tekton.WithVerbose(verbose), - tekton.WithPipelineDecorator(deployDecorator{}), + tekton.WithPipelineDecorator(deployDecorator{kc}), tekton.WithTransport(transport), } - return tekton.NewPipelinesProvider(options...) + return tekton.NewPipelinesProvider(kc, options...) } -func newKnativeDeployer(verbose bool) fn.Deployer { - options := []knative.DeployerOpt{ +func newKnativeDeployer(kc *k8s.Client, verbose bool) fn.Deployer { + return knative.NewDeployer(kc, knative.WithDeployerVerbose(verbose), - knative.WithDeployerDecorator(deployDecorator{}), - } - - return knative.NewDeployer(options...) + knative.WithDeployerDecorator(deployDecorator{kc}), + ) } // newK8sDeployer builds the raw deployer. @@ -181,10 +193,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 +206,30 @@ func newK8sDeployer(verbose bool) fn.Deployer { // bypassing it. Attached unconditionally for the reason in newK8sDeployer, // which bites harder here: keda's Route has no owner reference, so a Route // nothing goes looking for is a Route nothing ever removes. -func newKedaDeployer(verbose bool) fn.Deployer { - return keda.NewDeployer( +func newKedaDeployer(kc *k8s.Client, verbose bool) fn.Deployer { + return keda.NewDeployer(kc, keda.WithDeployerVerbose(verbose), - keda.WithDeployerDecorator(deployDecorator{}), + keda.WithDeployerDecorator(deployDecorator{kc}), keda.WithExposer(ocproute.New(deployers.Keda)), ) } +// deployDecorator adds OpenShift metadata when the target cluster is +// OpenShift. type deployDecorator struct { - oshDec k8s.OpenshiftMetadataDecorator + kc *k8s.Client } func (d deployDecorator) UpdateAnnotations(function fn.Function, annotations map[string]string) map[string]string { - if k8s.IsOpenShift() { - return d.oshDec.UpdateAnnotations(function, annotations) + if ok, _ := d.kc.IsOpenShift(); ok { + return k8s.OpenshiftMetadataDecorator{}.UpdateAnnotations(function, annotations) } return annotations } func (d deployDecorator) UpdateLabels(function fn.Function, labels map[string]string) map[string]string { - if k8s.IsOpenShift() { - return d.oshDec.UpdateLabels(function, labels) + if ok, _ := d.kc.IsOpenShift(); ok { + return k8s.OpenshiftMetadataDecorator{}.UpdateLabels(function, labels) } return labels } 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/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/deploy.go b/cmd/deploy.go index 37020c9e20..89745dc711 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) @@ -353,14 +357,14 @@ 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() + 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) } @@ -886,7 +890,7 @@ func (c deployConfig) clientOptions() ([]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) @@ -911,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 a32ac823be..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) } @@ -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/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/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/logs.go b/cmd/logs.go index b5207240c9..323fd874c9 100644 --- a/cmd/logs.go +++ b/cmd/logs.go @@ -19,8 +19,9 @@ 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, knative.GetKServiceLogs) @@ -99,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 @@ -197,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/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_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 aad8c367a7..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) } @@ -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) } @@ -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) } @@ -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) } @@ -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 baaad2ea77..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) } @@ -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", @@ -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.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(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) } @@ -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/functions/client_int_test.go b/pkg/functions/client_int_test.go index 9f47dee8d5..a5ce4fc8a2 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.NewClientFromKubeconfig() 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) } @@ -666,16 +667,16 @@ 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), 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), ) @@ -683,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), @@ -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..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 -func WithOpenShiftServiceCA() Option { +// 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 @@ -21,7 +26,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..795569aa6f 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(kc, fnhttp.WithOpenShiftServiceCA(kc)) defer transport.Close() client := http.Client{ diff --git a/pkg/http/transport.go b/pkg/http/transport.go index 6424d766e0..87ed9d9f23 100644 --- a/pkg/http/transport.go +++ b/pkg/http/transport.go @@ -50,18 +50,24 @@ 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). -func NewRoundTripper(opts ...Option) RoundTripCloser { +// This is useful for accessing cluster internal services (pushing a CloudEvent +// into Knative broker). +func NewRoundTripper(kc *k8s.Client, opts ...Option) RoundTripCloser { o := options{ - inClusterDialer: k8s.NewLazyInitInClusterDialer(k8s.GetClientConfig()), insecureSkipVerify: false, } for _, option := range opts { option(&o) } + // 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) + } httpTransport := newHTTPTransport() @@ -129,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 } @@ -145,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 ee6b9e744f..22f956eb1d 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() @@ -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 be45474cb4..3658e1b2a5 100644 --- a/pkg/k8s/client.go +++ b/pkg/k8s/client.go @@ -1,15 +1,18 @@ package k8s import ( + "errors" "fmt" "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,84 +20,179 @@ 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. +// +// 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 clientcmd.ClientConfig - cfg *rest.Config - cfgErr error - o sync.Once + // cc is the kubeconfig loader: merged files, contexts, active namespace. + // Nil when the Client was built from a rest.Config. + cc clientcmd.ClientConfig + + // cfg is the rest configuration given to NewClientFromConfig. + // Nil when the Client was built from a kubeconfig loader. + cfg *rest.Config + + // isOpenShift answers IsOpenShift; detection runs once per Client. + isOpenShift func() (bool, error) } +// 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") + +// 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") + +// 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 kubeconfig loader. func NewClient(cc clientcmd.ClientConfig) *Client { - return &Client{cc: cc} + return newClient(&Client{cc: cc}) } +// NewClientFromKubeconfig returns a Client which resolves its configuration +// the way kubectl does: KUBECONFIG, ~/.kube/config, in-cluster. +func NewClientFromKubeconfig() *Client { + return NewClient(kubeconfigClientConfig()) +} + +// NewClientFromConfig returns a Client backed by an already-resolved rest +// 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 &Client{cfg: cfg} + return newClient(&Client{cfg: cfg}) } -func (c *Client) ClientConfig() (*rest.Config, error) { - c.o.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 +func newClient(c *Client) *Client { + c.isOpenShift = sync.OnceValues(func() (bool, error) { return detectOpenShift(c) }) + return c } -func (c *Client) Clientset() (*kubernetes.Clientset, error) { - cfg, err := c.ClientConfig() +// 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) { + 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, err + return nil, fmt.Errorf("failed to create kubernetes client config: %w", err) } - return kubernetes.NewForConfig(cfg) + return cfg, nil } -func NewClientAndResolvedNamespace(ns string) (*kubernetes.Clientset, string, error) { - var err error - if ns == "" { - ns, err = GetDefaultNamespace() - if err != nil { - return nil, ns, err - } +// RawConfig returns the merged kubeconfig. +func (c *Client) RawConfig() (clientcmdapi.Config, error) { + if c == nil { + return clientcmdapi.Config{}, errNilClient } - - client, err := NewKubernetesClientset() - return client, ns, err + if c.cc == nil { + return clientcmdapi.Config{}, errNoKubeconfig + } + return c.cc.RawConfig() } -func NewKubernetesClientset() (*kubernetes.Clientset, error) { - restConfig, err := GetClientConfig().ClientConfig() +// Clientset returns a typed clientset. +func (c *Client) Clientset() (*kubernetes.Clientset, error) { + cfg, err := c.RestConfig() if err != nil { return nil, fmt.Errorf("failed to create new kubernetes client: %w", err) } - - return kubernetes.NewForConfig(restConfig) + return kubernetes.NewForConfig(cfg) } -func NewDynamicClient() (dynamic.Interface, error) { - restConfig, err := GetClientConfig().ClientConfig() +// 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 == nil { + return "", errNilClient + } + if c.cc == nil { + return "", errNoKubeconfig + } + 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 +} - return dynamic.NewForConfig(restConfig) +// 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) { + if c == nil { + return false, errNilClient + } + return c.isOpenShift() } -// GetDefaultNamespace returns default namespace -func GetDefaultNamespace() (namespace string, err error) { - namespace, _, err = GetClientConfig().Namespace() - return +// detectOpenShift asks the API server for the route.openshift.io/v1 group, +// 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 { + 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 + } } -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 5f96d61aea..845301019c 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,72 @@ 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) +// 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.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") + 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.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 := NewClientFromConfig(cfg) + c := k8s.NewClient(inMemoryLoader("https://example.com:6443")) cs, err := c.Clientset() if err != nil { @@ -37,20 +85,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 +127,116 @@ 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_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") + } +} + +// 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/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 8df73b6972..9d34a100bf 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) } @@ -190,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) } @@ -250,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) } @@ -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) } @@ -786,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" @@ -800,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) } @@ -815,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/deployer_int_test.go b/pkg/k8s/deployer_int_test.go index b2d466f927..bf0641305d 100644 --- a/pkg/k8s/deployer_int_test.go +++ b/pkg/k8s/deployer_int_test.go @@ -10,69 +10,76 @@ import ( ) func TestInt_FullPath(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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..29eddecc0e 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.NewClientFromKubeconfig() 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/dialer.go b/pkg/k8s/dialer.go index 5fe559c794..73ae9b9c60 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,7 +251,7 @@ 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 } @@ -269,7 +268,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 +283,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 +291,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 +433,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 +555,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 +570,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..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(clientConfig) + dialer := k8s.NewLazyInitInClusterDialer(kc) 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) } @@ -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, 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 24747a80e6..307cb240d3 100644 --- a/pkg/k8s/lister_int_test.go +++ b/pkg/k8s/lister_int_test.go @@ -10,11 +10,11 @@ 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(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/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..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) } @@ -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) } @@ -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) } @@ -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/openshift.go b/pkg/k8s/openshift.go index f47258618c..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" @@ -25,8 +23,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,8 +86,10 @@ func GetOpenShiftServiceCA(ctx context.Context) (*x509.Certificate, error) { } } -func GetDefaultOpenShiftRegistry() string { - ns, _ := GetDefaultNamespace() +// DefaultOpenShiftRegistry returns the internal registry path for the active +// namespace. +func (c *Client) DefaultOpenShiftRegistry() string { + ns, _ := c.DefaultNamespace() if ns == "" { ns = "default" } @@ -101,10 +103,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 } @@ -131,62 +133,6 @@ func GetOpenShiftDockerCredentialLoaders() []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 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 266af6a260..381a8187d7 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,17 +75,16 @@ 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) } @@ -102,7 +101,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 +117,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 +125,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 +269,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 +299,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/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..445ca19d62 100644 --- a/pkg/k8s/remover_int_test.go +++ b/pkg/k8s/remover_int_test.go @@ -10,11 +10,11 @@ import ( ) func TestInt_Remove(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() 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/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/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..303d61f3cd 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,17 +106,21 @@ 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) } // 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 { @@ -152,6 +159,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 +224,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 +465,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..fde810f405 100644 --- a/pkg/keda/deployer_int_test.go +++ b/pkg/keda/deployer_int_test.go @@ -11,69 +11,76 @@ import ( ) func TestInt_FullPath(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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..2a73a89318 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.NewClientFromKubeconfig() 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/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.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..916902af21 100644 --- a/pkg/keda/lister_int_test.go +++ b/pkg/keda/lister_int_test.go @@ -11,11 +11,11 @@ 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(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..4bb61a8ec4 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) } @@ -57,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 9b658e607a..b46bfed4f7 100644 --- a/pkg/keda/remover_int_test.go +++ b/pkg/keda/remover_int_test.go @@ -11,11 +11,11 @@ import ( ) func TestInt_Remove(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() 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..3992407521 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(kc, + fnhttp.WithOpenShiftServiceCA(kc), + fnhttp.WithInsecureSkipVerify(f.RegistryInsecure), + fnhttp.WithInClusterDialer(k8s.NewLazyInitInClusterDialer(kc)), + ) defer func(t fnhttp.RoundTripCloser) { _ = t.Close() }(t) @@ -198,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, @@ -224,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 @@ -328,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/deployer_int_test.go b/pkg/knative/deployer_int_test.go index 3d3801d0e4..986041dd74 100644 --- a/pkg/knative/deployer_int_test.go +++ b/pkg/knative/deployer_int_test.go @@ -11,67 +11,74 @@ import ( ) func TestInt_FullPath(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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.NewClientFromKubeconfig() 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..2b36e57701 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.NewClientFromKubeconfig() 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..43af901667 100644 --- a/pkg/knative/lister_int_test.go +++ b/pkg/knative/lister_int_test.go @@ -11,11 +11,11 @@ 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(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/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/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..6b7a842759 100644 --- a/pkg/knative/remover_int_test.go +++ b/pkg/knative/remover_int_test.go @@ -11,11 +11,11 @@ import ( ) func TestInt_Remove(t *testing.T) { - kc := k8s.NewClient(k8s.GetClientConfig()) + kc := k8s.NewClientFromKubeconfig() 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/operator/sync.go b/pkg/operator/sync.go index c067abe005..2855d2100a 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -43,8 +43,8 @@ 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 { - restCfg, err := k8s.GetClientConfig().ClientConfig() +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) } @@ -64,10 +64,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 +84,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/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 52bb3dac5a..4a041e6991 100644 --- a/pkg/pipelines/tekton/pipelines_int_test.go +++ b/pkg/pipelines/tekton/pipelines_int_test.go @@ -45,23 +45,23 @@ 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(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(kc, tekton.WithCredentialsProvider(testCP), tekton.WithVerbose(verbose))), } 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...)...) @@ -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) } @@ -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 87e3bd2fd6..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 == "" { @@ -63,7 +66,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 @@ -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) @@ -134,12 +140,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 } @@ -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 { @@ -201,19 +207,19 @@ 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 } 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 ef937b29a8..82affe6652 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 { @@ -90,8 +91,11 @@ func WithPacURLCallback(getPacURL pacURLCallback) Opt { } } -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{ @@ -116,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: @@ -167,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 } @@ -181,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 } @@ -190,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) { @@ -228,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) } @@ -262,19 +269,19 @@ 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) } 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) @@ -419,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) } @@ -441,7 +451,7 @@ 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, k8s.DeleteSecrets, @@ -456,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 } @@ -490,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 } @@ -537,7 +547,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. @@ -552,7 +562,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 } @@ -605,7 +615,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 != "" { @@ -613,7 +623,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.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 cff5fa33e3..8cce17953b 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,12 +37,12 @@ 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 -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/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) } }) 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) } 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 +}