diff --git a/cmd/build.go b/cmd/build.go index f7f4e25822..1512ec34d2 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,8 +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) + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + + f.Registry = resolveRegistry(f.Registry, "", kc) + // Client - clientOptions, err := cfg.clientOptions() + clientOptions, err := cfg.clientOptions(kc) if err != nil { return } @@ -264,8 +270,9 @@ func newBuildConfig() buildConfig { return buildConfig{ Global: config.Global{ Builder: viper.GetString("builder"), + Cluster: viper.GetString("cluster"), Confirm: viper.GetBool("confirm"), - Registry: registry(), // deferred defaulting + Registry: viper.GetString("registry"), Verbose: viper.GetBool("verbose"), RegistryInsecure: viper.GetBool("registry-insecure"), }, @@ -441,14 +448,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 +463,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 c4acf0802f..4bd3683812 100644 --- a/cmd/client.go +++ b/cmd/client.go @@ -32,6 +32,9 @@ type ClientConfig struct { // Allow insecure server connections when using SSL InsecureSkipVerify bool + + // Constructed k8s client config to be used with optional overrides + K8sClient *k8s.Client } // ClientFactory defines a constructor which assists in the creation of a Client @@ -45,6 +48,9 @@ type ClientFactory func(ClientConfig, ...fn.Option) (*fn.Client, func()) // NewTestClient returns a client factory which will ignore options used, // instead using those provided when creating the factory. This allows // for tests to create an entirely default client but with N mocks. +// +// gauron99: this could take *k8s.Client as parameter for easy testing if eg. +// testing cmd function's full flow with fake openshift cluster is desired func NewTestClient(options ...fn.Option) ClientFactory { return func(_ ClientConfig, _ ...fn.Option) (*fn.Client, func()) { return fn.New(options...), func() {} @@ -60,23 +66,24 @@ 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 ( - 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(cfg.Verbose), k8s.NewLister(cfg.Verbose), keda.NewLister(cfg.Verbose)), + fn.WithListers(knative.NewLister(kc, cfg.Verbose), k8s.NewLister(kc, cfg.Verbose), keda.NewLister(kc, cfg.Verbose)), fn.WithDeployer(d), fn.WithPipelinesProvider(pp), fn.WithPusher(docker.NewPusher( @@ -84,7 +91,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))), } ) @@ -103,10 +110,28 @@ func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) { return client, cleanup } +// newK8sClient is a convenient method of initializing k8s client +func newK8sClient(in *k8s.Client) *k8s.Client { + if in != nil { + return in + } + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + return k8s.NewClient(cc) +} + +// newK8sClientFromConfig builds a *k8s.Client from explicit cluster parameters. +func newK8sClientFromConfig(cluster, token, namespace string, local fn.Local) (*k8s.Client, error) { + cc, err := k8s.BuildClientConfig(cluster, token, namespace, local) + if err != nil { + return nil, err + } + return k8s.NewClient(cc), nil +} + // 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 @@ -114,8 +139,8 @@ func newTransport(insecureSkipVerify bool) fnhttp.RoundTripCloser { // of features or configuration nuances of cluster variants. // If authFilePath is provided (non-empty), it will be used as the primary auth file. // When insecure is true, credential verification uses plain HTTP instead of HTTPS. -func newCredentialsProvider(configPath string, t http.RoundTripper, authFilePath string, insecure bool) oci.CredentialsProvider { - additionalLoaders := append(k8s.GetOpenShiftDockerCredentialLoaders(), k8s.GetGoogleCredentialLoader()...) +func newCredentialsProvider(k8sClient *k8s.Client, configPath string, t http.RoundTripper, authFilePath string, insecure bool) oci.CredentialsProvider { + additionalLoaders := append(k8s.GetOpenShiftDockerCredentialLoaders(k8sClient), k8s.GetGoogleCredentialLoader()...) additionalLoaders = append(additionalLoaders, k8s.GetECRCredentialLoader()...) additionalLoaders = append(additionalLoaders, k8s.GetACRCredentialLoader()...) @@ -152,57 +177,58 @@ 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{k8sClient: kc}), tekton.WithTransport(transport), } - return tekton.NewPipelinesProvider(options...) + return tekton.NewPipelinesProvider(kc, options...) } -func newKnativeDeployer(verbose bool) fn.Deployer { +func newKnativeDeployer(kc *k8s.Client, verbose bool) fn.Deployer { options := []knative.DeployerOpt{ knative.WithDeployerVerbose(verbose), - knative.WithDeployerDecorator(deployDecorator{}), + knative.WithDeployerDecorator(deployDecorator{k8sClient: kc}), } - return knative.NewDeployer(options...) + return knative.NewDeployer(kc, options...) } -func newK8sDeployer(verbose bool) fn.Deployer { +func newK8sDeployer(kc *k8s.Client, verbose bool) fn.Deployer { options := []k8s.DeployerOpt{ k8s.WithDeployerVerbose(verbose), - k8s.WithDeployerDecorator(deployDecorator{}), + k8s.WithDeployerDecorator(deployDecorator{k8sClient: kc}), } - return k8s.NewDeployer(options...) + return k8s.NewDeployer(kc, options...) } -func newKedaDeployer(verbose bool) fn.Deployer { +func newKedaDeployer(kc *k8s.Client, verbose bool) fn.Deployer { options := []keda.DeployerOpt{ keda.WithDeployerVerbose(verbose), - keda.WithDeployerDecorator(deployDecorator{}), + keda.WithDeployerDecorator(deployDecorator{k8sClient: kc}), } - return keda.NewDeployer(options...) + return keda.NewDeployer(kc, options...) } type deployDecorator struct { - oshDec k8s.OpenshiftMetadataDecorator + k8sClient *k8s.Client + oshDec k8s.OpenshiftMetadataDecorator } func (d deployDecorator) UpdateAnnotations(function fn.Function, annotations map[string]string) map[string]string { - if k8s.IsOpenShift() { + if d.k8sClient != nil && d.k8sClient.IsOpenshift() { return d.oshDec.UpdateAnnotations(function, annotations) } return annotations } func (d deployDecorator) UpdateLabels(function fn.Function, labels map[string]string) map[string]string { - if k8s.IsOpenShift() { + if d.k8sClient != nil && d.k8sClient.IsOpenshift() { return d.oshDec.UpdateLabels(function, labels) } return labels diff --git a/cmd/completion_util.go b/cmd/completion_util.go index cffb092534..2979e35fd2 100644 --- a/cmd/completion_util.go +++ b/cmd/completion_util.go @@ -16,9 +16,11 @@ import ( ) func CompleteFunctionList(cmd *cobra.Command, args []string, toComplete string) (strings []string, directive cobra.ShellCompDirective) { + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) listers := []fn.Lister{ - knative.NewLister(false), - k8s.NewLister(false), + knative.NewLister(kc, false), + k8s.NewLister(kc, false), } items := []fn.ListItem{} diff --git a/cmd/config.go b/cmd/config.go index f5e1615728..42f57411b8 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -97,14 +97,19 @@ func runConfigCmd(cmd *cobra.Command, args []string) (err error) { if err != nil { return } + // construct k8s client for some commands do cluster search + kc, err := newK8sClientFromConfig(function.Deploy.Cluster, "", function.Deploy.Namespace, function.Local) + if err != nil { + return + } switch answers.SelectedOperation { case "Add": switch answers.SelectedConfig { case "Volumes": - err = runAddVolumesPrompt(cmd.Context(), function) + err = runAddVolumesPrompt(cmd.Context(), kc, function) case "Environment variables": - err = runAddEnvsPrompt(cmd.Context(), function) + err = runAddEnvsPrompt(cmd.Context(), kc, 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..5e34c927e3 100644 --- a/cmd/config_envs.go +++ b/cmd/config_envs.go @@ -130,7 +130,11 @@ set environment variable from a secret return loadSaver.Save(function) } - return runAddEnvsPrompt(cmd.Context(), function) + kc, err := newK8sClientFromConfig(function.Deploy.Cluster, "", function.Deploy.Namespace, function.Local) + if err != nil { + return err + } + return runAddEnvsPrompt(cmd.Context(), kc, function) }, } @@ -211,7 +215,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 +247,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..cb3310ac3c 100644 --- a/cmd/config_volumes.go +++ b/cmd/config_volumes.go @@ -98,7 +98,11 @@ For non-interactive usage, use flags to specify the volume type and configuratio } // Fall back to interactive mode - return runAddVolumesPrompt(cmd.Context(), function) + kc, err := newK8sClientFromConfig(function.Deploy.Cluster, "", function.Deploy.Namespace, function.Local) + if err != nil { + return err + } + return runAddVolumesPrompt(cmd.Context(), kc, function) }, } @@ -164,17 +168,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/delete.go b/cmd/delete.go index 103a6270c4..5b4058f1e0 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -33,7 +33,7 @@ No local files are deleted. SuggestFor: []string{"remove", "del"}, Aliases: []string{"rm"}, ValidArgsFunction: CompleteFunctionList, - PreRunE: bindEnv("path", "confirm", "all", "namespace", "verbose"), + PreRunE: bindEnv("cluster", "cluster-token", "path", "confirm", "all", "namespace", "verbose"), SilenceUsage: true, // no usage dump on error RunE: func(cmd *cobra.Command, args []string) error { // Layer 2: Catch technical errors and provide CLI-specific user-friendly messages @@ -47,7 +47,15 @@ No local files are deleted. fmt.Fprintf(cmd.OutOrStdout(), "error loading config at '%v'. %v\n", config.File(), err) } + // Function Context + f, _ := fn.NewFunction(effectivePath()) + if f.Initialized() { + cfg = cfg.Apply(f) + } + // Flags + cmd.Flags().String("cluster", cfg.Cluster, "Specify a cluster api url for your function deployment. ($FUNC_CLUSTER)") + cmd.Flags().String("cluster-token", "", "Bearer token for cluster authentication. ($FUNC_CLUSTER_TOKEN)") cmd.Flags().StringP("namespace", "n", defaultNamespace(fn.Function{}, false), "The namespace when deleting by name. ($FUNC_NAMESPACE)") cmd.Flags().StringP("all", "a", "true", "Delete all resources created for a function, eg. Pipelines, Secrets, etc. ($FUNC_ALL) (allowed values: \"true\", \"false\")") addConfirmFlag(cmd, cfg.Confirm) @@ -78,26 +86,43 @@ func runDelete(cmd *cobra.Command, args []string, newClient ClientFactory) (err return } - client, done := newClient(ClientConfig{Verbose: cfg.Verbose}) - defer done() - if cfg.Name != "" { // Delete by name if provided - return client.Remove(cmd.Context(), cfg.Name, cfg.Namespace, fn.Function{}, cfg.All) - } else { // Otherwise; delete the function at path (cwd by default) - f, err := fn.NewFunction(cfg.Path) + // don't use local.yaml auth because we are not concerned about func at path + kc, err := newK8sClientFromConfig(cfg.Cluster, cfg.ClusterToken, cfg.Namespace, fn.Local{}) if err != nil { return err } - return client.Remove(cmd.Context(), "", "", f, cfg.All) + + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, K8sClient: kc}) + defer done() + return client.Remove(cmd.Context(), cfg.Name, cfg.Namespace, fn.Function{}, cfg.All) } + + // Delete by path + f, err := fn.NewFunction(cfg.Path) + if err != nil { + return err + } + + // delete by-path targets the function's state unless specifically overridden + kc, err := newK8sClientFromConfig(cfg.Cluster, cfg.ClusterToken, f.Deploy.Namespace, f.Local) + if err != nil { + return err + } + + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, K8sClient: kc}) + defer done() + return client.Remove(cmd.Context(), "", "", f, cfg.All) } type deleteConfig struct { - Name string - Namespace string - Path string - All bool - Verbose bool + Cluster string + ClusterToken string + Name string + Namespace string + Path string + All bool + Verbose bool } // newDeleteConfig returns a config populated from the current execution context @@ -108,11 +133,13 @@ func newDeleteConfig(cmd *cobra.Command, args []string) (cfg deleteConfig, err e name = args[0] } cfg = deleteConfig{ - All: viper.GetBool("all"), - Name: name, // args[0] or derived - Namespace: viper.GetString("namespace"), - Path: viper.GetString("path"), - Verbose: viper.GetBool("verbose"), // defined on root + All: viper.GetBool("all"), + Cluster: viper.GetString("cluster"), + ClusterToken: viper.GetString("cluster-token"), + Name: name, // args[0] or derived + Namespace: viper.GetString("namespace"), + Path: viper.GetString("path"), + Verbose: viper.GetBool("verbose"), // defined on root } if cfg.Name == "" && cmd.Flags().Changed("namespace") { // logicially inconsistent to supply only a namespace. diff --git a/cmd/deploy.go b/cmd/deploy.go index e5a1a8af29..df25470329 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -130,11 +130,12 @@ EXAMPLES `, SuggestFor: []string{"delpoy", "deplyo"}, PreRunE: bindEnv("build", "build-timestamp", "builder", "builder-image", - "base-image", "confirm", "domain", "env", "git-branch", "git-dir", - "git-url", "image", "image-pull-secret", "management-disabled", "namespace", "path", "platform", "push", "pvc-size", - "service-account", "deployer", "registry", "registry-insecure", - "registry-authfile", "remote", "username", "password", "token", "verbose", - "remote-storage-class"), + "base-image", "cluster", "cluster-token", "save-cluster-auth", + "confirm", "domain", "env", "git-branch", "git-dir", "git-url", + "image", "image-pull-secret", "management-disabled", "namespace", + "path", "platform", "push", "pvc-size", "service-account", "deployer", + "registry", "registry-insecure", "registry-authfile", "remote", + "username", "password", "token", "verbose", "remote-storage-class"), RunE: func(cmd *cobra.Command, args []string) error { return runDeploy(cmd, newClient) }, @@ -159,6 +160,10 @@ EXAMPLES // contextually relevant function; but sets are flattened via cfg.Apply(f) cmd.Flags().StringP("builder", "b", cfg.Builder, fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s.", KnownBuilders())) + cmd.Flags().String("cluster", cfg.Cluster, "Specify a cluster api url for your function deployment. ($FUNC_CLUSTER)") + cmd.Flags().String("cluster-token", "", "Bearer token for cluster authentication. Persisted to .func/local.yaml on successful deploy. ($FUNC_CLUSTER_TOKEN)") + cmd.Flags().Bool("save-cluster-auth", true, + "Persist resolved cluster credentials to .func/local.yaml after a successful deploy so later deploys reach the same cluster regardless of the active kubeconfig context. Use --save-cluster-auth=false to deploy without storing credentials. ($FUNC_SAVE_CLUSTER_AUTH)") cmd.Flags().StringP("registry", "r", cfg.Registry, "Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)") cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)") @@ -281,41 +286,33 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { return wrapValidateError(err, "deploy") } - // Warn if registry changed but registryInsecure is still true + // Informative warnings about config changes (must be before Configure + // which overwrites the old values on the function struct) warnRegistryInsecureChange(cmd.OutOrStderr(), cfg.Registry, f) + warnDeployTargetChange(cmd.OutOrStderr(), cfg, f) if f, err = cfg.Configure(f); err != nil { // Updates f with deploy cfg return } - changingNamespace := func(f fn.Function) bool { - // We're changing namespace if: - return f.Deploy.Namespace != "" && // it's already deployed - f.Namespace != "" && // a specific (new) namespace is requested - (f.Namespace != f.Deploy.Namespace) // and it's different + // construct k8s client to be used on every cluster interaction + kc, err := newK8sClientFromConfig(cfg.Cluster, cfg.ClusterToken, cfg.Namespace, f.Local) + if err != nil { + return } - // If we're changing namespace in an OpenShift cluster, we have to - // 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) { - 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) - } - } + f.Registry = resolveRegistry(f.Registry, f.Namespace, kc) // Informative non-error messages regarding the final deployment request - printDeployMessages(cmd.OutOrStdout(), f) + printDeployMessages(cmd.OutOrStdout(), f, kc) // Get options based on the value of the config such as concrete impls // of builders and pushers based on the value of the --builder flag - 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 @@ -391,6 +388,33 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { } } + // After a successful deploy, pin the resolved cluster into func.yaml and — + // unless --save-cluster-auth=false — persist its creds to .func/local.yaml + // (never source-controlled) so later deploys reach the same cluster + // regardless of the active kubeconfig context. + if f.Local.FindAuth(f.Deploy.Cluster) == nil { + if url, clusterTLS, user, extractErr := kc.Auth(); extractErr == nil { + if f.Deploy.Cluster == "" { + f.Deploy.Cluster = url // pin the target cluster + } + if cfg.SaveClusterAuth { + if cfg.ClusterToken != "" { + user.Token = cfg.ClusterToken + } + f.Local.SetAuth(f.Deploy.Cluster, clusterTLS, user) + fmt.Fprintf(cmd.OutOrStderr(), "Cached cluster credentials for '%s' in .func/local.yaml (not source controlled).\n", f.Deploy.Cluster) + } + } + } else if cfg.SaveClusterAuth && cfg.ClusterToken != "" { + // Merge explicit --cluster-token into existing stored auth + if entry := f.Local.FindAuth(f.Deploy.Cluster); entry != nil { + clusterTLS := entry.Cluster + user := entry.User + user.Token = cfg.ClusterToken + f.Local.SetAuth(f.Deploy.Cluster, clusterTLS, user) + } + } + // Write if err = f.Write(); err != nil { return @@ -473,6 +497,15 @@ func KnownBuilders() builders.Known { type deployConfig struct { buildConfig // further embeds config.Global + // ClusterToken is a bearer token for authenticating to the deployment + // cluster. When set, it takes precedence over stored credentials. + ClusterToken string + + // SaveClusterAuth controls whether resolved cluster credentials are + // persisted to .func/local.yaml after a successful deploy. Defaults to true; + // set flag to false (or FUNC_SAVE_CLUSTER_AUTH=false) to deploy without storing. + SaveClusterAuth bool + // Perform build using the settings from the embedded buildConfig struct. // Acceptable values are the keyword 'auto', or a truthy value such as // 'true', 'false, '1' or '0'. @@ -540,7 +573,7 @@ type deployConfig struct { // ImagePullSecret is the name of a secret for pulling the function image ImagePullSecret string - // Deployer specifies the type of deployment: "knative" or "raw" + // Deployer specifies the type of deployment: "knative","raw" or "keda" Deployer string // Remote indicates the deployment (and possibly build) process are to @@ -567,6 +600,8 @@ type deployConfig struct { func newDeployConfig(cmd *cobra.Command) deployConfig { cfg := deployConfig{ buildConfig: newBuildConfig(), + ClusterToken: viper.GetString("cluster-token"), + SaveClusterAuth: viper.GetBool("save-cluster-auth"), Build: viper.GetString("build"), Env: viper.GetStringSlice("env"), Domain: viper.GetString("domain"), @@ -790,19 +825,19 @@ 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 deployer := c.Deployer @@ -812,11 +847,11 @@ func (c deployConfig) clientOptions() ([]fn.Option, error) { switch 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)", deployer, knative.KnativeDeployerName, k8s.KubernetesDeployerName, keda.KedaDeployerName) } @@ -825,36 +860,17 @@ 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, f fn.Function, cc *k8s.Client) { + // Always inform the user where the function is being deployed + if restCfg, err := cc.ClientConfig(); err == nil && restCfg.Host != "" { + fmt.Fprintf(out, "Deploying function to cluster '%s' in namespace '%s'\n", restCfg.Host, f.Namespace) + } + 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) } - // Namespace - // --------- - currentNamespace := f.Deploy.Namespace // will be "" if no initialed f at path. - targetNamespace := f.Namespace - if targetNamespace == "" { - return - } - - // If creating a duplicate deployed function in a different - // namespace. - if targetNamespace != currentNamespace && currentNamespace != "" { - fmt.Fprintf(out, "Info: chosen namespace has changed from '%s' to '%s'. Undeploying function from '%s' and deploying new in '%s'.\n", currentNamespace, targetNamespace, currentNamespace, targetNamespace) - } - - // Namespace Changing - // ------------------- - // 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() - 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) - } - // Git Args // ----------------- // Print a warning if the function already contains Git attributes, but the @@ -894,6 +910,35 @@ func printDeployMessages(out io.Writer, f fn.Function) { } } +// resolveRegistry returns the registry to use for the function. +// - If no registry is set, checks for OpenShift and uses its internal registry. +// - If an OpenShift internal registry is set and namespace is provided, +// rewrites the registry to include the target namespace. +// - Otherwise returns the registry as-is. +func resolveRegistry(registry, namespace string, c *k8s.Client) string { + if registry == "" { + if c.IsOpenshift() { + return k8s.GetDefaultOpenShiftRegistry(c) + } + return "" + } + if namespace != "" && c.IsOpenshift() && k8s.IsOpenShiftInternalRegistry(registry) { + return "image-registry.openshift-image-registry.svc:5000/" + namespace + } + return registry +} + +// warnDeployTargetChange informs the user when the deploy target (cluster or +// namespace) is changing from a previous deployment. +func warnDeployTargetChange(w io.Writer, cfg deployConfig, f fn.Function) { + if cfg.Cluster != "" && f.Deploy.Cluster != "" && cfg.Cluster != f.Deploy.Cluster { + fmt.Fprintf(w, "Warning: Changing deployment cluster from '%s' to '%s'. Function will NOT be removed from the old cluster.\n", f.Deploy.Cluster, cfg.Cluster) + } + if f.Deploy.Namespace != "" && cfg.Namespace != f.Deploy.Namespace { + fmt.Fprintf(w, "Info: namespace changed from '%s' to '%s'. Function will be moved.\n", f.Deploy.Namespace, cfg.Namespace) + } +} + // isDigested checks that the given image reference has a digest. Invalid // reference return error. func isDigested(v string) (validDigest bool, err error) { diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index 540cb4e15f..46d6aad292 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -8,7 +8,6 @@ import ( "reflect" "strings" "testing" - "time" "github.com/ory/viper" "github.com/spf13/cobra" @@ -18,6 +17,9 @@ import ( "knative.dev/func/pkg/k8s" "knative.dev/func/pkg/mock" . "knative.dev/func/pkg/testing" + + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) // commandConstructor is used to share test implementations between commands @@ -1065,6 +1067,84 @@ func TestDeploy_Namespace(t *testing.T) { } } +// writeKubeconfigNS points KUBECONFIG at a single-context kubeconfig whose +// active context defaults to the given namespace -- i.e. simulates `kubens `. +func writeKubeconfigNS(t *testing.T, namespace string) { + t.Helper() + path := filepath.Join(t.TempDir(), "kubeconfig") + cfg := clientcmdapi.Config{ + CurrentContext: "test", + Contexts: map[string]*clientcmdapi.Context{"test": {Cluster: "test", AuthInfo: "test", Namespace: namespace}}, + Clusters: map[string]*clientcmdapi.Cluster{"test": {Server: "https://cluster.example.com:6443", InsecureSkipTLSVerify: true}}, + AuthInfos: map[string]*clientcmdapi.AuthInfo{"test": {Token: "test-token"}}, + } + if err := clientcmd.WriteToFile(cfg, path); err != nil { + t.Fatal(err) + } + t.Setenv("KUBECONFIG", path) +} + +// TestDeploy_NamespacePinSurvivesContextSwitch is the headline guarantee: once a +// function is deployed, switching the active kubeconfig context (the `kubens` +// case) must NOT move where it deploys. Complements TestDeploy_Namespace, which +// keeps the active context constant; here the active namespace actively changes +// between deploys. +func TestDeploy_NamespacePinSurvivesContextSwitch(t *testing.T) { + root := FromTempDirectory(t) + testClientFn := NewTestClient(fn.WithDeployer(mock.NewDeployer())) + + f, err := fn.New().Init(fn.Function{Root: root, Runtime: "go", Registry: TestRegistry}) + if err != nil { + t.Fatal(err) + } + + // Deploy pins the function to "team-a". + cmd := NewDeployCmd(testClientFn) + cmd.SetArgs([]string{"--namespace=team-a"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + if f, _ = fn.NewFunction(root); f.Deploy.Namespace != "team-a" { + t.Fatalf("setup: expected deployed namespace 'team-a', got %q", f.Deploy.Namespace) + } + + // kubens: switch the active context to a different namespace. + writeKubeconfigNS(t, "switched-ns") + + // Redeploy with no --namespace: the function's pin must win over the now + // different active context, not follow it to 'switched-ns'. + cmd = NewDeployCmd(testClientFn) + cmd.SetArgs([]string{}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + if f, _ = fn.NewFunction(root); f.Deploy.Namespace != "team-a" { + t.Fatalf("namespace pin did not survive context switch: got %q, want team-a", f.Deploy.Namespace) + } +} + +// TestDeploy_NamespaceFromEnv verifies the env-var leg of namespace resolution +// (FUNC_NAMESPACE), mirroring the env coverage cluster/token already have. +func TestDeploy_NamespaceFromEnv(t *testing.T) { + root := FromTempDirectory(t) + t.Setenv("FUNC_NAMESPACE", "env-ns") // after FromTempDirectory, which clears FUNC_* + testClientFn := NewTestClient(fn.WithDeployer(mock.NewDeployer())) + + f, err := fn.New().Init(fn.Function{Root: root, Runtime: "go", Registry: TestRegistry}) + if err != nil { + t.Fatal(err) + } + + cmd := NewDeployCmd(testClientFn) + cmd.SetArgs([]string{}) // no --namespace; env should win over the active-context default + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + if f, _ = fn.NewFunction(root); f.Deploy.Namespace != "env-ns" { + t.Fatalf("expected namespace from FUNC_NAMESPACE 'env-ns', got %q", f.Deploy.Namespace) + } +} + // TestDeploy_NamespaceDefaultsToK8sContext ensures that when not specified, a // users's active kubernetes context is used for the namespace if available. func TestDeploy_NamespaceDefaultsToK8sContext(t *testing.T) { @@ -1165,20 +1245,10 @@ func TestDeploy_NamespaceRedeployWarning(t *testing.T) { fn.WithRegistry(TestRegistry), )) cmd.SetArgs([]string{}) - stdout := strings.Builder{} - cmd.SetOut(&stdout) if err := cmd.Execute(); err != nil { t.Fatal(err) } - expected := "Warning: namespace chosen is 'funcns', but currently active namespace is 'mynamespace'. Continuing with deployment to 'funcns'." - - // Ensure output contained warning if changing namespace - if !strings.Contains(stdout.String(), expected) { - t.Log("STDOUT:\n" + stdout.String()) - t.Fatalf("Expected warning not found:\n%v", expected) - } - // Ensure the function was saved as having been deployed to the correct ns f, err = fn.NewFunction(root) if err != nil { @@ -1193,15 +1263,22 @@ func TestDeploy_NamespaceRedeployWarning(t *testing.T) { // to a new namespace issues a warning. // Also implicitly checks that the --namespace flag takes precedence over // the namespace of a previously deployed Function. -func TestDeploy_NamespaceUpdateWarning(t *testing.T) { +// TestDeploy_DeployTargetChangeWarning ensures that changing namespace or +// cluster from a previous deployment issues an informational message. +func TestDeploy_DeployTargetChangeWarning(t *testing.T) { root := FromTempDirectory(t) + // Provide a kubeconfig so BuildClientConfig can resolve auth for the new cluster + writeDeployTestKubeconfig(t, "https://new-cluster:6443", "test-token") + // Create a Function which appears to have been deployed to 'myns' + // on cluster 'https://old-cluster:6443' f := fn.Function{ Runtime: "go", Root: root, Deploy: fn.DeploySpec{ Namespace: "myns", + Cluster: "https://old-cluster:6443", }, } f, err := fn.New().Init(f) @@ -1209,16 +1286,15 @@ func TestDeploy_NamespaceUpdateWarning(t *testing.T) { t.Fatal(err) } - // Redeploy the function, specifying 'newns' + // Redeploy the function, specifying new namespace and cluster cmd := NewDeployCmd(NewTestClient( fn.WithDeployer(mock.NewDeployer()), fn.WithBuilder(mock.NewBuilder()), fn.WithPipelinesProvider(mock.NewPipelinesProvider()), fn.WithRegistry(TestRegistry), )) - cmd.SetArgs([]string{"--namespace=newns"}) + cmd.SetArgs([]string{"--namespace=newns", "--cluster=https://new-cluster:6443"}) out := strings.Builder{} - fmt.Fprintln(&out, "Test error") cmd.SetOut(&out) cmd.SetErr(&out) @@ -1226,28 +1302,357 @@ func TestDeploy_NamespaceUpdateWarning(t *testing.T) { t.Fatal(err) } - time.Sleep(1 * time.Second) + output := out.String() - activeNamespace, err := k8s.GetDefaultNamespace() - if err != nil { - t.Fatalf("Couldn't get active namespace, got error: %v", err) + // Ensure output contains namespace change info + expectedNS := "Info: namespace changed from 'myns' to 'newns'. Function will be moved." + if !strings.Contains(output, expectedNS) { + t.Log("OUTPUT:\n" + output) + t.Fatalf("Expected namespace change info not found:\n%v", expectedNS) } - expected1 := "Info: chosen namespace has changed from 'myns' to 'newns'. Undeploying function from 'myns' and deploying new in 'newns'." - expected2 := fmt.Sprintf("Warning: namespace chosen is 'newns', but currently active namespace is '%s'. Continuing with deployment to 'newns'.", activeNamespace) - // Ensure output contained info and warning if changing namespace - if !strings.Contains(out.String(), expected1) || !strings.Contains(out.String(), expected2) { - t.Log("STDERR:\n" + out.String()) - t.Fatalf("Expected Info and/or Warning not found:\n%v|%v", expected1, expected2) + // Ensure output contains cluster change info + expectedCluster := "Warning: Changing deployment cluster from 'https://old-cluster:6443' to 'https://new-cluster:6443'. Function will NOT be removed from the old cluster." + if !strings.Contains(output, expectedCluster) { + t.Log("OUTPUT:\n" + output) + t.Fatalf("Expected cluster change info not found:\n%v", expectedCluster) } - // Ensure the function was saved as having been deployed to + // Ensure the function was saved with the new namespace f, err = fn.NewFunction(root) if err != nil { t.Fatal(err) } if f.Deploy.Namespace != "newns" { - t.Fatalf("expected function to be deployed into namespace 'newns'. got '%v'", f.Deploy.Namespace) + t.Fatalf("expected function to be deployed into namespace 'newns'. got '%v'", f.Deploy.Namespace) + } + if f.Deploy.Cluster != "https://new-cluster:6443" { + t.Fatalf("expected function to be deployed to cluster 'https://new-cluster:6443'. got '%v'", f.Deploy.Cluster) + } +} + +// TestDeploy_ClusterAuthPriorityFlow walks the full cluster/auth resolution +// ladder and asserts which source wins at each step. +// Priority, lowest first: +// +// 1. global config default +// 2. stored credential (.func/local.yaml) +// 3. FUNC_CLUSTER(_TOKEN) env +// 4. --cluster/--cluster-token flag +// +// Each step is checked by reading back the pinned cluster (func.yaml +// deploy.cluster) and the cached credential (.func/local.yaml). Auth resolution +// uses the real k8s client, so the stored token proves which (url, token) resolved. +func TestDeploy_ClusterAuthPriorityFlow(t *testing.T) { + root := FromTempDirectory(t) + // Global config default supplies cluster: https://cfg-cluster:6443 (+ registry) + t.Setenv("XDG_CONFIG_HOME", fmt.Sprintf("%s/testdata/TestDeploy_ClusterAuthPriorityFlow", cwd())) + + clientFn := NewTestClient( + fn.WithBuilder(mock.NewBuilder()), + fn.WithPusher(mock.NewPusher()), + fn.WithDeployer(mock.NewDeployer()), + fn.WithPipelinesProvider(mock.NewPipelinesProvider()), + fn.WithRegistry(TestRegistry), + ) + + if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root, Name: "f"}); err != nil { + t.Fatal(err) + } + + deploy := func(args ...string) { + t.Helper() + cmd := NewDeployCmd(clientFn) // fresh per step to avoid viper/flag carryover + cmd.SetArgs(args) + var out strings.Builder + cmd.SetOut(&out) + cmd.SetErr(&out) + if err := cmd.Execute(); err != nil { + t.Fatalf("deploy %v: %v\noutput:\n%s", args, err, out.String()) + } + } + reload := func() fn.Function { + t.Helper() + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + return f + } + tokenFor := func(f fn.Function, url string) string { + t.Helper() + e := f.Local.FindAuth(url) + if e == nil { + t.Fatalf("no stored auth entry for %q; entries=%+v", url, f.Local.Auth) + } + return e.User.Token + } + + // Loopback URLs so the IsOpenshift discovery call is refused instantly + // (an unreachable hostname would wait on a multi-second DNS timeout). + const ( + cfgCluster = "https://127.0.0.1:6443" + envCluster = "https://127.0.0.1:6444" + flagCluster = "https://127.0.0.1:6445" + ) + + // STEP 1 — global config default pins the cluster and caches kubeconfig auth. + writeDeployTestKubeconfig(t, cfgCluster, "cfg-token") + deploy() + if f := reload(); f.Deploy.Cluster != cfgCluster { + t.Fatalf("step1: pin = %q, want %q", f.Deploy.Cluster, cfgCluster) + } else if len(f.Local.Auth) != 1 || tokenFor(f, cfgCluster) != "cfg-token" { + t.Fatalf("step1: want 1 entry cfg-token, got %+v", f.Local.Auth) + } + + // STEP 2 — redeploy, no flags, kubeconfig now serves a STALE token for the + // same cluster: the STORED credential must win (FindAuth short-circuits the + // kubeconfig search), the pin is unchanged, and no second entry is appended. + writeDeployTestKubeconfig(t, cfgCluster, "STALE") + deploy() + if f := reload(); f.Deploy.Cluster != cfgCluster { + t.Fatalf("step2: pin changed to %q, want %q", f.Deploy.Cluster, cfgCluster) + } else if len(f.Local.Auth) != 1 || tokenFor(f, cfgCluster) != "cfg-token" { + t.Fatalf("step2: stored creds did not beat kubeconfig; got %+v", f.Local.Auth) + } + + // STEP 3 — FUNC_CLUSTER(_TOKEN) env wins over the stored func.yaml pin. + t.Setenv("FUNC_CLUSTER", envCluster) + t.Setenv("FUNC_CLUSTER_TOKEN", "env-token") + deploy() + if f := reload(); f.Deploy.Cluster != envCluster { + t.Fatalf("step3: env did not win; pin = %q, want %q", f.Deploy.Cluster, envCluster) + } else if len(f.Local.Auth) != 2 { + t.Fatalf("step3: want 2 entries, got %+v", f.Local.Auth) + } else if tokenFor(f, envCluster) != "env-token" { + t.Fatalf("step3: env token = %q, want env-token", tokenFor(f, envCluster)) + } else if tokenFor(f, cfgCluster) != "cfg-token" { + t.Fatalf("step3: cfg entry mutated to %q", tokenFor(f, cfgCluster)) + } + + // STEP 4 — explicit --cluster/--cluster-token flag wins over env. + deploy("--cluster="+flagCluster, "--cluster-token=flag-token") + if f := reload(); f.Deploy.Cluster != flagCluster { + t.Fatalf("step4: flag did not win; pin = %q, want %q", f.Deploy.Cluster, flagCluster) + } else if len(f.Local.Auth) != 3 || tokenFor(f, flagCluster) != "flag-token" { + t.Fatalf("step4: want flag-token entry, got %+v", f.Local.Auth) + } +} + +// authTestClient returns a deploy ClientFactory whose build/push/deploy are all +// mocked so the deploy succeeds and the real credential-persistence block runs +// without a cluster. (The k8s client used for auth resolution is still the real +// one, so persisted .func/local.yaml reflects what was actually resolved.) +func authTestClient() ClientFactory { + return NewTestClient( + fn.WithBuilder(mock.NewBuilder()), + fn.WithPusher(mock.NewPusher()), + fn.WithDeployer(mock.NewDeployer()), + fn.WithPipelinesProvider(mock.NewPipelinesProvider()), + fn.WithRegistry(TestRegistry), + ) +} + +// TestDeploy_SaveAuthGate asserts --save-cluster-auth (and +// FUNC_SAVE_CLUSTER_AUTH): the default caches credentials, while +// --save-cluster-auth=false pins the cluster but writes none. +func TestDeploy_SaveAuthGate(t *testing.T) { + const cluster = "https://127.0.0.1:6443" + run := func(root string, args ...string) fn.Function { + t.Helper() + cmd := NewDeployCmd(authTestClient()) + cmd.SetArgs(args) + var out strings.Builder + cmd.SetOut(&out) + cmd.SetErr(&out) + if err := cmd.Execute(); err != nil { + t.Fatalf("deploy %v: %v\n%s", args, err, out.String()) + } + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + return f + } + initFn := func(root string) { + t.Helper() + writeDeployTestKubeconfig(t, cluster, "kube-token") + if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root, Name: "f"}); err != nil { + t.Fatal(err) + } + } + + // default: cluster pinned AND credentials cached + root := FromTempDirectory(t) + initFn(root) + if f := run(root, "--cluster="+cluster); f.Deploy.Cluster != cluster { + t.Fatalf("default: cluster not pinned, got %q", f.Deploy.Cluster) + } else if e := f.Local.FindAuth(cluster); e == nil || e.User.Token != "kube-token" { + t.Fatalf("default: want cached kube-token, got %+v", f.Local.Auth) + } + + // --save-cluster-auth=false: cluster pinned, NO credentials written + root = FromTempDirectory(t) + initFn(root) + if f := run(root, "--cluster="+cluster, "--save-cluster-auth=false"); f.Deploy.Cluster != cluster { + t.Fatalf("flag-off: cluster not pinned, got %q", f.Deploy.Cluster) + } else if len(f.Local.Auth) != 0 { + t.Fatalf("flag-off: expected no stored auth, got %+v", f.Local.Auth) + } + + // FUNC_SAVE_CLUSTER_AUTH=false behaves like the flag (set AFTER FromTempDirectory so + // it survives the helper's env clearing) + root = FromTempDirectory(t) + t.Setenv("FUNC_SAVE_CLUSTER_AUTH", "false") + initFn(root) + if f := run(root, "--cluster="+cluster); f.Deploy.Cluster != cluster { + t.Fatalf("env-off: cluster not pinned, got %q", f.Deploy.Cluster) + } else if len(f.Local.Auth) != 0 { + t.Fatalf("env-off: expected no stored auth, got %+v", f.Local.Auth) + } +} + +// TestDeploy_KubeconfigFallback_PinsAndStores asserts the "old way": with no +// --cluster/env/stored auth, the active kubeconfig is used, and the resolved +// cluster + auth are then pinned/cached for subsequent deploys. +func TestDeploy_KubeconfigFallback_PinsAndStores(t *testing.T) { + const cluster = "https://127.0.0.1:6443" + root := FromTempDirectory(t) + writeDeployTestKubeconfig(t, cluster, "kube-token") // the active-context source + if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root, Name: "f"}); err != nil { + t.Fatal(err) + } + + cmd := NewDeployCmd(authTestClient()) // NO --cluster / FUNC_CLUSTER / stored auth + var out strings.Builder + cmd.SetOut(&out) + cmd.SetErr(&out) + if err := cmd.Execute(); err != nil { + t.Fatalf("deploy: %v\n%s", err, out.String()) + } + + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if f.Deploy.Cluster != cluster { + t.Fatalf("expected cluster pinned from active kubeconfig %q, got %q", cluster, f.Deploy.Cluster) + } + if e := f.Local.FindAuth(cluster); e == nil || e.User.Token != "kube-token" { + t.Fatalf("expected active-context auth cached, got %+v", f.Local.Auth) + } +} + +// writeTwoContextKubeconfig points KUBECONFIG at a kubeconfig with an active +// context (activeServer/activeToken) and a second, non-active context +// (otherServer/otherToken). +func writeTwoContextKubeconfig(t *testing.T, activeServer, activeToken, otherServer, otherToken string) { + t.Helper() + path := filepath.Join(t.TempDir(), "kubeconfig") + cfg := clientcmdapi.Config{ + CurrentContext: "active", + Contexts: map[string]*clientcmdapi.Context{ + "active": {Cluster: "active-cluster", AuthInfo: "active-user"}, + "other": {Cluster: "other-cluster", AuthInfo: "other-user"}, + }, + Clusters: map[string]*clientcmdapi.Cluster{ + "active-cluster": {Server: activeServer}, + "other-cluster": {Server: otherServer}, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "active-user": {Token: activeToken}, + "other-user": {Token: otherToken}, + }, + } + if err := clientcmd.WriteToFile(cfg, path); err != nil { + t.Fatal(err) + } + t.Setenv("KUBECONFIG", path) +} + +// TestDeploy_ClusterFlagResolvesNonActiveContext covers the "exactly one +// non-active match" rung of kubeconfig resolution at the cmd level: deploying +// with --cluster pointing at a cluster that is NOT the active context, but is +// targeted by exactly one other context, must resolve and persist THAT +// context's token -- not the active context's. (The deploy itself is mocked; +// kc.Auth() is a local read, so neither cluster needs to be reachable.) +func TestDeploy_ClusterFlagResolvesNonActiveContext(t *testing.T) { + const ( + activeCluster = "https://127.0.0.1:6443" // current-context + otherCluster = "https://127.0.0.1:6444" // a non-active context targets this + ) + root := FromTempDirectory(t) + writeTwoContextKubeconfig(t, activeCluster, "active-token", otherCluster, "other-token") + if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root, Name: "f"}); err != nil { + t.Fatal(err) + } + + cmd := NewDeployCmd(authTestClient()) + cmd.SetArgs([]string{"--cluster=" + otherCluster}) // non-active cluster, no token, no stored auth + var out strings.Builder + cmd.SetOut(&out) + cmd.SetErr(&out) + if err := cmd.Execute(); err != nil { + t.Fatalf("deploy: %v\n%s", err, out.String()) + } + + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if f.Deploy.Cluster != otherCluster { + t.Fatalf("pin = %q, want %q", f.Deploy.Cluster, otherCluster) + } + e := f.Local.FindAuth(otherCluster) + if e == nil { + t.Fatalf("no stored auth for %q; entries=%+v", otherCluster, f.Local.Auth) + } + if e.User.Token != "other-token" { + t.Fatalf("stored token = %q, want other-token (the non-active matched context, not active-token)", e.User.Token) + } +} + +// TestDeploy_ClusterTokenUpdatesStoredTokenPreservesCA asserts the merge branch: +// redeploying with a new --cluster-token updates the stored token while leaving +// the previously-stored cluster CA intact. +func TestDeploy_ClusterTokenUpdatesStoredTokenPreservesCA(t *testing.T) { + const cluster = "https://127.0.0.1:6443" + root := FromTempDirectory(t) + writeDeployTestKubeconfig(t, cluster, "ignored") + + f := fn.Function{Runtime: "go", Root: root, Name: "f", Deploy: fn.DeploySpec{Cluster: cluster}} + f, err := fn.New().Init(f) + if err != nil { + t.Fatal(err) + } + // seed a stored entry: token v1 + a cluster CA + f.Local.SetAuth(cluster, fn.ClusterTLS{CertificateAuthorityData: "CA-DATA"}, fn.UserAuth{Token: "tok-v1"}) + if err := f.Write(); err != nil { + t.Fatal(err) + } + + cmd := NewDeployCmd(authTestClient()) + cmd.SetArgs([]string{"--cluster=" + cluster, "--cluster-token=tok-v2"}) + var out strings.Builder + cmd.SetOut(&out) + cmd.SetErr(&out) + if err := cmd.Execute(); err != nil { + t.Fatalf("deploy: %v\n%s", err, out.String()) + } + + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + e := f.Local.FindAuth(cluster) + if e == nil { + t.Fatalf("stored entry disappeared; entries=%+v", f.Local.Auth) + } + if e.User.Token != "tok-v2" { + t.Fatalf("token not updated: want tok-v2, got %q", e.User.Token) + } + if e.Cluster.CertificateAuthorityData != "CA-DATA" { + t.Fatalf("stored CA not preserved across --cluster-token update: got %q", e.Cluster.CertificateAuthorityData) } } @@ -1340,72 +1745,32 @@ func TestDeploy_BasicRedeployPipelinesCorrectNamespace(t *testing.T) { // TestDeploy_NamespaceChangePreservesExternalRegistry ensures that changing // namespace on OpenShift does not overwrite an external registry (e.g. // docker.io/user) with the internal OpenShift registry. -// Regression test for https://github.com/knative/func/issues/2172 -func TestDeploy_NamespaceChangePreservesExternalRegistry(t *testing.T) { - root := FromTempDirectory(t) - - cleanup := k8s.SetOpenShiftForTest(true) - defer cleanup() - - // Create a function deployed to "ns1" with an external registry - f := fn.Function{ - Runtime: "go", - Root: root, - Registry: "docker.io/user", - Deploy: fn.DeploySpec{Namespace: "ns1"}, - } - f, err := fn.New().Init(f) - if err != nil { - t.Fatal(err) - } - - // Deploy to a different namespace - cmd := NewDeployCmd(NewTestClient(fn.WithDeployer(mock.NewDeployer()))) - cmd.SetArgs([]string{"--namespace=ns2"}) - if err := cmd.Execute(); err != nil { - t.Fatal(err) - } - - // Reload and verify the external registry was preserved - f, _ = fn.NewFunction(root) - if f.Registry != "docker.io/user" { - t.Errorf("expected registry 'docker.io/user' to be preserved, got %q", f.Registry) - } -} - -// TestDeploy_NamespaceChangeUpdatesInternalRegistry ensures that changing -// namespace on OpenShift DOES update the registry when the function uses -// the internal OpenShift registry (the namespace is part of the registry path). -func TestDeploy_NamespaceChangeUpdatesInternalRegistry(t *testing.T) { - root := FromTempDirectory(t) - - cleanup := k8s.SetOpenShiftForTest(true) - defer cleanup() - - // Create a function deployed to "ns1" using the internal registry - f := fn.Function{ - Runtime: "go", - Root: root, - Registry: "image-registry.openshift-image-registry.svc:5000/ns1", - Deploy: fn.DeploySpec{Namespace: "ns1"}, - } - f, err := fn.New().Init(f) - if err != nil { - t.Fatal(err) - } +// +// If running full deploy cmd flow here is desired, we might consider changing +// signature of NewTestClient() function (noted at the functions' constructor) +func TestResolveRegistry_OpenShift(t *testing.T) { + fakeLocal := fn.Local{Auth: []fn.AuthEntry{{ClusterURL: "fake-cluster", User: fn.UserAuth{Token: "fake"}}}} + fakeCC, _ := k8s.BuildClientConfig("fake-cluster", "", "", fakeLocal) + cc := k8s.NewClientWithOpenShift(fakeCC, true) - // Deploy to a different namespace - cmd := NewDeployCmd(NewTestClient(fn.WithDeployer(mock.NewDeployer()))) - cmd.SetArgs([]string{"--namespace=ns2"}) - if err := cmd.Execute(); err != nil { - t.Fatal(err) + tests := []struct { + name string + registry string + namespace string + expected string + }{ + {"external registry preserved", "docker.io/user", "ns2", "docker.io/user"}, + {"internal registry rewritten", "image-registry.openshift-image-registry.svc:5000/ns1", "ns2", "image-registry.openshift-image-registry.svc:5000/ns2"}, + {"empty fallback to openshift", "", "", "image-registry.openshift-image-registry.svc:5000/default"}, } - // Reload and verify the internal registry was updated to the new namespace - f, _ = fn.NewFunction(root) - expected := "image-registry.openshift-image-registry.svc:5000/ns2" - if f.Registry != expected { - t.Errorf("expected registry to update to %q, got %q", expected, f.Registry) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ret := resolveRegistry(tt.registry, tt.namespace, cc) + if ret != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, ret) + } + }) } } @@ -2547,3 +2912,19 @@ func TestDeploy_ValidDomain(t *testing.T) { func TestDeploy_RegistryInsecurePersists(t *testing.T) { testRegistryInsecurePersists(NewDeployCmd, t) } + +func writeDeployTestKubeconfig(t *testing.T, serverURL, token string) { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "kubeconfig") + cfg := clientcmdapi.Config{ + CurrentContext: "test", + Contexts: map[string]*clientcmdapi.Context{"test": {Cluster: "test", AuthInfo: "test"}}, + Clusters: map[string]*clientcmdapi.Cluster{"test": {Server: serverURL}}, + AuthInfos: map[string]*clientcmdapi.AuthInfo{"test": {Token: token}}, + } + if err := clientcmd.WriteToFile(cfg, path); err != nil { + t.Fatal(err) + } + t.Setenv("KUBECONFIG", path) +} diff --git a/cmd/describe.go b/cmd/describe.go index 5a6f4a8d49..dc076e5505 100644 --- a/cmd/describe.go +++ b/cmd/describe.go @@ -34,7 +34,7 @@ the current directory or from the directory specified with --path. ValidArgsFunction: CompleteFunctionList, Aliases: []string{"info", "desc"}, - PreRunE: bindEnv("output", "path", "namespace", "verbose"), + PreRunE: bindEnv("cluster", "cluster-token", "output", "path", "namespace", "verbose"), RunE: func(cmd *cobra.Command, args []string) error { return runDescribe(cmd, args, newClient) }, @@ -46,7 +46,15 @@ the current directory or from the directory specified with --path. fmt.Fprintf(cmd.OutOrStdout(), "error loading config at '%v'. %v\n", config.File(), err) } + // Function Context + f, _ := fn.NewFunction(effectivePath()) + if f.Initialized() { + cfg = cfg.Apply(f) + } + // Flags + cmd.Flags().String("cluster", cfg.Cluster, "Specify a cluster api url for your function deployment. ($FUNC_CLUSTER)") + cmd.Flags().String("cluster-token", "", "Bearer token for cluster authentication. ($FUNC_CLUSTER_TOKEN)") cmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|yaml|url) ($FUNC_OUTPUT)") cmd.Flags().StringP("namespace", "n", defaultNamespace(fn.Function{}, false), "The namespace in which to look for the named function. ($FUNC_NAMESPACE)") addPathFlag(cmd) @@ -66,11 +74,16 @@ func runDescribe(cmd *cobra.Command, args []string, newClient ClientFactory) (er } // TODO cfg.Prompt() - client, done := newClient(ClientConfig{Verbose: cfg.Verbose}) - defer done() - var details fn.Instance if cfg.Name != "" { // Describe by name if provided + // don't use local.yaml auth because we are not concerned about func at path + kc, err := newK8sClientFromConfig(cfg.Cluster, cfg.ClusterToken, cfg.Namespace, fn.Local{}) + if err != nil { + return err + } + + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, K8sClient: kc}) + defer done() details, err = client.Describe(cmd.Context(), cfg.Name, cfg.Namespace, fn.Function{}) if err != nil { return err @@ -83,6 +96,16 @@ func runDescribe(cmd *cobra.Command, args []string, newClient ClientFactory) (er if !f.Initialized() { return NewErrNotInitializedFromPath(f.Root, "describe") } + + // By-path describe targets the function's pinned namespace (Describe + // resolves f.Deploy.Namespace), so pin the client to it, not active. + kc, err := newK8sClientFromConfig(cfg.Cluster, cfg.ClusterToken, f.Deploy.Namespace, f.Local) + if err != nil { + return err + } + + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, K8sClient: kc}) + defer done() details, err = client.Describe(cmd.Context(), "", "", f) if err != nil { return err @@ -97,11 +120,13 @@ func runDescribe(cmd *cobra.Command, args []string, newClient ClientFactory) (er // ------------------------------ type describeConfig struct { - Name string - Namespace string - Output string - Path string - Verbose bool + Cluster string + ClusterToken string + Name string + Namespace string + Output string + Path string + Verbose bool } func newDescribeConfig(cmd *cobra.Command, args []string) (cfg describeConfig, err error) { @@ -110,11 +135,13 @@ func newDescribeConfig(cmd *cobra.Command, args []string) (cfg describeConfig, e name = args[0] } cfg = describeConfig{ - Name: name, - Namespace: viper.GetString("namespace"), - Output: viper.GetString("output"), - Path: viper.GetString("path"), - Verbose: viper.GetBool("verbose"), + Cluster: viper.GetString("cluster"), + ClusterToken: viper.GetString("cluster-token"), + Name: name, + Namespace: viper.GetString("namespace"), + Output: viper.GetString("output"), + Path: viper.GetString("path"), + Verbose: viper.GetBool("verbose"), } if cfg.Name == "" && cmd.Flags().Changed("namespace") { // logically inconsistent to supply only a namespace. diff --git a/cmd/environment.go b/cmd/environment.go index 31aee04f31..f4677ea75b 100644 --- a/cmd/environment.go +++ b/cmd/environment.go @@ -13,7 +13,7 @@ import ( "knative.dev/func/pkg/buildpacks" "knative.dev/func/pkg/config" - "knative.dev/func/pkg/functions" + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" "knative.dev/func/pkg/pipelines/tekton" "knative.dev/func/pkg/s2i" @@ -72,8 +72,8 @@ type Environment struct { Environment []string Cluster string Defaults config.Global - Function *functions.Function `json:",omitempty" yaml:",omitempty"` - Instance *functions.Instance `json:",omitempty" yaml:",omitempty"` + Function *fn.Function `json:",omitempty" yaml:",omitempty"` + Instance *fn.Instance `json:",omitempty" yaml:",omitempty"` } func runEnvironment(cmd *cobra.Command, newClient ClientFactory, v *Version) (err error) { @@ -83,7 +83,7 @@ func runEnvironment(cmd *cobra.Command, newClient ClientFactory, v *Version) (er } // Create a client to get runtimes and templates - client := functions.New(functions.WithVerbose(cfg.Verbose)) + client := fn.New(fn.WithVerbose(cfg.Verbose)) r, err := getRuntimes(client) if err != nil { @@ -114,13 +114,16 @@ func runEnvironment(cmd *cobra.Command, newClient ClientFactory, v *Version) (er return } - // Gets the cluster host + f, _ := fn.NewFunction(cfg.Path) + + // Gets the cluster host — use the function's stored cluster if available var host string - cc, err := k8s.GetClientConfig().ClientConfig() - if err != nil { - fmt.Printf("error getting client config %v\n", err) - } else { - host = cc.Host + envKc, kcErr := newK8sClientFromConfig(f.Deploy.Cluster, "", f.Deploy.Namespace, f.Local) + if kcErr == nil { + restCfg, clientErr := envKc.ClientConfig() + if clientErr == nil { + host = restCfg.Host + } } //Get default image builders @@ -131,7 +134,7 @@ func runEnvironment(cmd *cobra.Command, newClient ClientFactory, v *Version) (er environment := Environment{ Version: v.String(), GitRevision: v.Hash, - SpecVersion: functions.LastSpecVersion(), + SpecVersion: fn.LastSpecVersion(), SocatImage: k8s.SocatImage, TarImage: k8s.TarImage, FuncUtilsImage: tekton.FuncUtilImage, @@ -146,12 +149,11 @@ func runEnvironment(cmd *cobra.Command, newClient ClientFactory, v *Version) (er Defaults: defaults, } - function, instance := describeFuncInformation(cmd.Context(), newClient, cfg) - if function != nil { - environment.Function = function - } - if instance != nil { - environment.Instance = instance + if f.Initialized() { + environment.Function = &f + if instance := describeFuncInformation(cmd.Context(), f, envKc, newClient, cfg); instance != nil { + environment.Instance = instance + } } var s []byte @@ -171,7 +173,7 @@ func runEnvironment(cmd *cobra.Command, newClient ClientFactory, v *Version) (er return nil } -func getRuntimes(client *functions.Client) ([]string, error) { +func getRuntimes(client *fn.Client) ([]string, error) { runtimes, err := client.Runtimes() if err != nil { return nil, err @@ -179,7 +181,7 @@ func getRuntimes(client *functions.Client) ([]string, error) { return runtimes, nil } -func getTemplates(client *functions.Client, runtimes []string) (map[string][]string, error) { +func getTemplates(client *fn.Client, runtimes []string) (map[string][]string, error) { templateMap := make(map[string][]string) for _, runtime := range runtimes { templates, err := client.Templates().List(runtime) @@ -191,20 +193,14 @@ func getTemplates(client *functions.Client, runtimes []string) (map[string][]str return templateMap, nil } -func describeFuncInformation(context context.Context, newClient ClientFactory, cfg environmentConfig) (*functions.Function, *functions.Instance) { - function, err := functions.NewFunction(cfg.Path) - if err != nil || !function.Initialized() { - return nil, nil - } - - client, done := newClient(ClientConfig{Verbose: cfg.Verbose}) +func describeFuncInformation(ctx context.Context, f fn.Function, kc *k8s.Client, newClient ClientFactory, cfg environmentConfig) *fn.Instance { + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, K8sClient: kc}) defer done() - - instance, err := client.Describe(context, function.Name, function.Deploy.Namespace, function) + instance, err := client.Describe(ctx, f.Name, f.Deploy.Namespace, f) if err != nil { - return &function, nil + return nil } - return &function, &instance + return &instance } type environmentConfig struct { diff --git a/cmd/errors.go b/cmd/errors.go index e820df1d96..a919cb7f28 100644 --- a/cmd/errors.go +++ b/cmd/errors.go @@ -267,10 +267,11 @@ func (e *ErrClusterNotAccessible) Error() string { Cannot connect to Kubernetes cluster. No valid cluster configuration found. Try this: - minikube start Start Minikube cluster - kind create cluster Start Kind cluster - kubectl cluster-info Verify cluster is running - kubectl config get-contexts List available contexts + func deploy --cluster --cluster-token Connect directly without kubeconfig + minikube start Start Minikube cluster + kind create cluster Start Kind cluster + kubectl cluster-info Verify cluster is running + kubectl config get-contexts List available contexts For more options, run 'func deploy --help'`, e.Err) } // end if diff --git a/cmd/func-util/main.go b/cmd/func-util/main.go index 360fcf38a7..b465e8df3e 100644 --- a/cmd/func-util/main.go +++ b/cmd/func-util/main.go @@ -156,21 +156,23 @@ func deploy(ctx context.Context) error { if f.Deploy.Deployer == "" { f.Deploy.Deployer = knative.KnativeDeployerName } + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) var d fn.Deployer switch f.Deploy.Deployer { case knative.KnativeDeployerName: - d = knative.NewDeployer( - knative.WithDeployerDecorator(deployDecorator{}), + d = knative.NewDeployer(kc, + knative.WithDeployerDecorator(deployDecorator{k8sClient: kc}), knative.WithDeployerVerbose(true), ) case k8s.KubernetesDeployerName: - d = k8s.NewDeployer( - k8s.WithDeployerDecorator(deployDecorator{}), + d = k8s.NewDeployer(kc, + k8s.WithDeployerDecorator(deployDecorator{k8sClient: kc}), k8s.WithDeployerVerbose(true), ) case keda.KedaDeployerName: - d = keda.NewDeployer( - keda.WithDeployerDecorator(deployDecorator{}), + d = keda.NewDeployer(kc, + keda.WithDeployerDecorator(deployDecorator{k8sClient: kc}), keda.WithDeployerVerbose(true), ) default: @@ -188,18 +190,19 @@ func deploy(ctx context.Context) error { } type deployDecorator struct { - oshDec k8s.OpenshiftMetadataDecorator + k8sClient *k8s.Client + oshDec k8s.OpenshiftMetadataDecorator } func (d deployDecorator) UpdateAnnotations(function fn.Function, annotations map[string]string) map[string]string { - if k8s.IsOpenShift() { + if d.k8sClient != nil && d.k8sClient.IsOpenshift() { return d.oshDec.UpdateAnnotations(function, annotations) } return annotations } func (d deployDecorator) UpdateLabels(function fn.Function, labels map[string]string) map[string]string { - if k8s.IsOpenShift() { + if d.k8sClient != nil && d.k8sClient.IsOpenshift() { return d.oshDec.UpdateLabels(function, labels) } return labels diff --git a/cmd/invoke.go b/cmd/invoke.go index 9ff96d37fa..744e49dd5c 100644 --- a/cmd/invoke.go +++ b/cmd/invoke.go @@ -106,7 +106,7 @@ EXAMPLES "onvoke", "unvoke", "knvoke", "imvoke", "ihvoke", "ibvoke"}, PreRunE: bindEnv("path", "format", "target", "id", "source", "type", "data", "content-type", "request-type", "file", "insecure", - "confirm", "verbose", "extension"), + "confirm", "verbose", "extension", "cluster", "cluster-token"), RunE: func(cmd *cobra.Command, args []string) error { return runInvoke(cmd, args, newClient) }, @@ -130,6 +130,8 @@ EXAMPLES cmd.Flags().StringP("file", "", "", "Path to a file to use as data. Overrides --data flag and should be sent with a correct --content-type. ($FUNC_FILE)") cmd.Flags().BoolP("insecure", "i", false, "Allow insecure server connections when using SSL. ($FUNC_INSECURE)") cmd.Flags().StringSliceP("extension", "e", nil, "Extensions as key=value pairs. Can be repeated. cloudevents only ($FUNC_EXTENSION)") + cmd.Flags().String("cluster", cfg.Cluster, "Specify a cluster api url. ($FUNC_CLUSTER)") + cmd.Flags().String("cluster-token", "", "Bearer token for cluster authentication. ($FUNC_CLUSTER_TOKEN)") addConfirmFlag(cmd, cfg.Confirm) addPathFlag(cmd) addVerboseFlag(cmd, cfg.Verbose) @@ -180,8 +182,16 @@ func runInvoke(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err } } - // Client instance from env vars, flags, args and user prompts (if --confirm) - client, done := newClient(ClientConfig{Verbose: cfg.Verbose, InsecureSkipVerify: cfg.Insecure}) + // Build k8s client for cluster access + cluster := cfg.Cluster + if cluster == "" { + cluster = f.Deploy.Cluster + } + kc, err := newK8sClientFromConfig(cluster, cfg.ClusterToken, f.Deploy.Namespace, f.Local) + if err != nil { + return err + } + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, InsecureSkipVerify: cfg.Insecure, K8sClient: kc}) defer done() // Message to send the running function built from parameters gathered @@ -242,38 +252,42 @@ func runInvoke(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err } type invokeConfig struct { - Path string - Target string - Format string - ID string - Source string - Type string - Data []byte - ContentType string - RequestType string - File string - Confirm bool - Verbose bool - Insecure bool - Extensions []string + Path string + Target string + Format string + ID string + Source string + Type string + Data []byte + ContentType string + RequestType string + File string + Cluster string + ClusterToken string + Confirm bool + Verbose bool + Insecure bool + Extensions []string } func newInvokeConfig() (cfg invokeConfig, err error) { cfg = invokeConfig{ - Path: viper.GetString("path"), - Target: viper.GetString("target"), - Format: viper.GetString("format"), - ID: viper.GetString("id"), - Source: viper.GetString("source"), - Type: viper.GetString("type"), - Data: []byte(viper.GetString("data")), - ContentType: viper.GetString("content-type"), - RequestType: viper.GetString("request-type"), - File: viper.GetString("file"), - Confirm: viper.GetBool("confirm"), - Verbose: viper.GetBool("verbose"), - Insecure: viper.GetBool("insecure"), - Extensions: viper.GetStringSlice("extension"), + Path: viper.GetString("path"), + Target: viper.GetString("target"), + Format: viper.GetString("format"), + ID: viper.GetString("id"), + Source: viper.GetString("source"), + Type: viper.GetString("type"), + Data: []byte(viper.GetString("data")), + ContentType: viper.GetString("content-type"), + RequestType: viper.GetString("request-type"), + File: viper.GetString("file"), + Cluster: viper.GetString("cluster"), + ClusterToken: viper.GetString("cluster-token"), + Confirm: viper.GetBool("confirm"), + Verbose: viper.GetBool("verbose"), + Insecure: viper.GetBool("insecure"), + Extensions: viper.GetStringSlice("extension"), } // If file was passed, read it in as data diff --git a/cmd/list.go b/cmd/list.go index 3e24e561ff..ecde3d8fd8 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -35,7 +35,7 @@ Lists deployed functions. `, SuggestFor: []string{"lsit"}, Aliases: []string{"ls"}, - PreRunE: bindEnv("all-namespaces", "output", "namespace", "verbose"), + PreRunE: bindEnv("all-namespaces", "output", "namespace", "cluster", "cluster-token", "verbose"), RunE: func(cmd *cobra.Command, args []string) error { return runList(cmd, args, newClient) }, @@ -68,6 +68,8 @@ Lists deployed functions. cmd.Flags().BoolP("all-namespaces", "A", false, "List functions in all namespaces. If set, the --namespace flag is ignored.") cmd.Flags().StringP("namespace", "n", defaultNamespace(fn.Function{}, false), "The namespace for which to list functions. ($FUNC_NAMESPACE)") cmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml) ($FUNC_OUTPUT)") + cmd.Flags().String("cluster", cfg.Cluster, "Specify a cluster api url. ($FUNC_CLUSTER)") + cmd.Flags().String("cluster-token", "", "Bearer token for cluster authentication. ($FUNC_CLUSTER_TOKEN)") addVerboseFlag(cmd, cfg.Verbose) if err := cmd.RegisterFlagCompletionFunc("output", CompleteOutputFormatList); err != nil { @@ -83,7 +85,11 @@ func runList(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error return err } - client, done := newClient(ClientConfig{Verbose: cfg.Verbose}) + kc, err := newK8sClientFromConfig(cfg.Cluster, cfg.ClusterToken, cfg.Namespace, fn.Local{}) + if err != nil { + return err + } + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, K8sClient: kc}) defer done() items, err := client.List(cmd.Context(), cfg.Namespace) @@ -105,16 +111,20 @@ func runList(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error // ------------------------------ type listConfig struct { - Namespace string - Output string - Verbose bool + Namespace string + Output string + Cluster string + ClusterToken string + Verbose bool } func newListConfig(cmd *cobra.Command) (cfg listConfig, err error) { cfg = listConfig{ - Namespace: viper.GetString("namespace"), - Output: viper.GetString("output"), - Verbose: viper.GetBool("verbose"), + Namespace: viper.GetString("namespace"), + Output: viper.GetString("output"), + Cluster: viper.GetString("cluster"), + ClusterToken: viper.GetString("cluster-token"), + Verbose: viper.GetBool("verbose"), } // If --all-namespaces, zero out any value for namespace (such as) // "all" to the lister. diff --git a/cmd/logs.go b/cmd/logs.go index 8a0bcb6135..59933ccfc7 100644 --- a/cmd/logs.go +++ b/cmd/logs.go @@ -40,7 +40,7 @@ specified with --path. Abstracts away the underlying service name and pod detail `, SuggestFor: []string{"log", "tail"}, ValidArgsFunction: CompleteFunctionList, - PreRunE: bindEnv("name", "namespace", "path", "since", "verbose"), + PreRunE: bindEnv("name", "namespace", "path", "since", "cluster", "cluster-token", "verbose"), RunE: func(cmd *cobra.Command, args []string) error { return runLogs(cmd, newClient) }, @@ -56,6 +56,8 @@ specified with --path. Abstracts away the underlying service name and pod detail cmd.Flags().StringP("name", "", "", "Name of the function to get logs from ($FUNC_NAME)") cmd.Flags().StringP("namespace", "n", defaultNamespace(fn.Function{}, false), "The namespace of the function ($FUNC_NAMESPACE)") cmd.Flags().StringP("since", "", "1m", "Return logs newer than a relative duration like 5s, 2m, or 3h ($FUNC_LOGS_SINCE)") + cmd.Flags().String("cluster", cfg.Cluster, "Specify a cluster api url. ($FUNC_CLUSTER)") + cmd.Flags().String("cluster-token", "", "Bearer token for cluster authentication. ($FUNC_CLUSTER_TOKEN)") addPathFlag(cmd) addVerboseFlag(cmd, cfg.Verbose) @@ -68,14 +70,16 @@ func runLogs(cmd *cobra.Command, newClient ClientFactory) error { return err } - client, done := newClient(ClientConfig{Verbose: cfg.Verbose}) - defer done() - // Get function details and deployer type var f fn.Function var deployer string if cfg.Name != "" { - // Get function by name + kc, err := newK8sClientFromConfig(cfg.Cluster, cfg.ClusterToken, cfg.Namespace, fn.Local{}) + if err != nil { + return err + } + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, K8sClient: kc}) + defer done() instance, err := client.Describe(cmd.Context(), cfg.Name, cfg.Namespace, fn.Function{}) if err != nil { return fmt.Errorf("failed to get function details: %w", err) @@ -85,7 +89,6 @@ func runLogs(cmd *cobra.Command, newClient ClientFactory) error { f.Image = instance.Image deployer = instance.Deployer } else { - // Load function from path f, err = fn.NewFunction(cfg.Path) if err != nil { return err @@ -94,7 +97,16 @@ func runLogs(cmd *cobra.Command, newClient ClientFactory) error { return NewErrNotInitializedFromPath(f.Root, "logs") } - // Get deployed function details to ensure it exists + cluster := cfg.Cluster + if cluster == "" { + cluster = f.Deploy.Cluster + } + kc, err := newK8sClientFromConfig(cluster, cfg.ClusterToken, f.Deploy.Namespace, f.Local) + if err != nil { + return err + } + client, done := newClient(ClientConfig{Verbose: cfg.Verbose, K8sClient: kc}) + defer done() instance, err := client.Describe(cmd.Context(), "", "", f) if err != nil { return fmt.Errorf("function not deployed or not found: %w", err) @@ -142,7 +154,15 @@ func runLogs(cmd *cobra.Command, newClient ClientFactory) error { fmt.Fprintf(os.Stderr, "Streaming logs for function '%s' in namespace '%s'...\n", f.Name, f.Namespace) fmt.Fprintf(os.Stderr, "Press Ctrl+C to stop.\n\n") - err = knative.GetKServiceLogs(ctx, f.Namespace, f.Name, f.Image, sinceTime, os.Stdout) + cluster := cfg.Cluster + if cluster == "" { + cluster = f.Deploy.Cluster + } + kc, err := newK8sClientFromConfig(cluster, cfg.ClusterToken, f.Namespace, f.Local) + if err != nil { + return err + } + err = knative.GetKServiceLogs(ctx, kc, f.Namespace, f.Name, f.Image, sinceTime, os.Stdout) if err != nil && err != context.Canceled { return fmt.Errorf("failed to stream logs: %w", err) } @@ -154,20 +174,24 @@ func runLogs(cmd *cobra.Command, newClient ClientFactory) error { // ------------------------------ type logsConfig struct { - Name string - Namespace string - Path string - Since string - Verbose bool + Name string + Namespace string + Path string + Since string + Cluster string + ClusterToken string + Verbose bool } func newLogsConfig(cmd *cobra.Command) (cfg logsConfig, err error) { cfg = logsConfig{ - Name: viper.GetString("name"), - Namespace: viper.GetString("namespace"), - Path: viper.GetString("path"), - Since: viper.GetString("since"), - Verbose: viper.GetBool("verbose"), + Name: viper.GetString("name"), + Namespace: viper.GetString("namespace"), + Path: viper.GetString("path"), + Cluster: viper.GetString("cluster"), + ClusterToken: viper.GetString("cluster-token"), + Since: viper.GetString("since"), + Verbose: viper.GetBool("verbose"), } if cfg.Name != "" && cmd.Flags().Changed("path") { diff --git a/cmd/root.go b/cmd/root.go index ec1e2e680f..3aec0e6d81 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -138,16 +138,6 @@ Learn more about Knative at: https://knative.dev`, cfg.Name), // Helpers // ------------------------------------------ -// registry to use is that provided as --registry or FUNC_REGISTRY. -// If not provided, global configuration determines the default to use. -func registry() string { - if r := viper.GetString("registry"); r != "" { - return r - } - cfg, _ := config.NewDefault() - return cfg.RegistryDefault() -} - // effectivePath to use is that which was provided by --path or FUNC_PATH. // Manually parses flags such that this can be used during (cobra/viper) flag // definition (prior to parsing). @@ -190,7 +180,9 @@ func defaultNamespace(f fn.Function, verbose bool) string { } // Active K8S namespace - namespace, err := k8s.GetDefaultNamespace() + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + defaultKc := k8s.NewClient(cc) + namespace, err := defaultKc.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 bea2c961cd..a21f234637 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" ) @@ -180,8 +181,11 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { cfg.Verbose = false } - // Client - clientOptions, err := cfg.clientOptions() + // construct a k8s client with default k8s configs because 'run' shares its + // function client options with build - buildConfig.clientOptions() + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + clientOptions, err := cfg.clientOptions(kc) if err != nil { return } diff --git a/cmd/testdata/TestDeploy_ClusterAuthPriorityFlow/func/config.yaml b/cmd/testdata/TestDeploy_ClusterAuthPriorityFlow/func/config.yaml new file mode 100644 index 0000000000..873da1c8f8 --- /dev/null +++ b/cmd/testdata/TestDeploy_ClusterAuthPriorityFlow/func/config.yaml @@ -0,0 +1,2 @@ +registry: example.com/alice +cluster: https://127.0.0.1:6443 diff --git a/docs/reference/func_delete.md b/docs/reference/func_delete.md index c22a87a692..52247bec9a 100644 --- a/docs/reference/func_delete.md +++ b/docs/reference/func_delete.md @@ -32,12 +32,14 @@ func delete myfunc --namespace apps ### Options ``` - -a, --all string Delete all resources created for a function, eg. Pipelines, Secrets, etc. ($FUNC_ALL) (allowed values: "true", "false") (default "true") - -c, --confirm Prompt to confirm options interactively ($FUNC_CONFIRM) - -h, --help help for delete - -n, --namespace string The namespace when deleting by name. ($FUNC_NAMESPACE) (default "default") - -p, --path string Path to the function. Default is current directory ($FUNC_PATH) - -v, --verbose Print verbose logs ($FUNC_VERBOSE) + -a, --all string Delete all resources created for a function, eg. Pipelines, Secrets, etc. ($FUNC_ALL) (allowed values: "true", "false") (default "true") + --cluster string Specify a cluster api url for your function deployment. ($FUNC_CLUSTER) + --cluster-token string Bearer token for cluster authentication. ($FUNC_CLUSTER_TOKEN) + -c, --confirm Prompt to confirm options interactively ($FUNC_CONFIRM) + -h, --help help for delete + -n, --namespace string The namespace when deleting by name. ($FUNC_NAMESPACE) (default "default") + -p, --path string Path to the function. Default is current directory ($FUNC_PATH) + -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` ### SEE ALSO diff --git a/docs/reference/func_deploy.md b/docs/reference/func_deploy.md index 81880ce1c4..4c212ab560 100644 --- a/docs/reference/func_deploy.md +++ b/docs/reference/func_deploy.md @@ -118,6 +118,8 @@ func deploy --build-timestamp Use the actual time as the created time for the docker image. This is only useful for buildpacks builder. -b, --builder string Builder to use when creating the function's container. Currently supported builders are "host", "pack" and "s2i". (default "pack") --builder-image string Specify a custom builder image for use by the builder other than its default. ($FUNC_BUILDER_IMAGE) + --cluster string Specify a cluster api url for your function deployment. ($FUNC_CLUSTER) + --cluster-token string Bearer token for cluster authentication. Persisted to .func/local.yaml on successful deploy. ($FUNC_CLUSTER_TOKEN) -c, --confirm Prompt to confirm options interactively ($FUNC_CONFIRM) --deployer string Type of deployment to use: 'knative' for Knative Service (default), 'raw' for Kubernetes Deployment or 'keda' for Deployment with a Keda HTTP scaler ($FUNC_DEPLOY_TYPE) --domain string Domain to use for the function's route. Cluster must be configured with domain matching for the given domain (ignored if unrecognized) ($FUNC_DOMAIN) @@ -140,6 +142,7 @@ func deploy --registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE) -R, --remote Trigger a remote deployment. Default is to deploy and build from the local system ($FUNC_REMOTE) --remote-storage-class string Specify a storage class to use for the volume on-cluster during remote builds + --save-cluster-auth Persist resolved cluster credentials to .func/local.yaml after a successful deploy so later deploys reach the same cluster regardless of the active kubeconfig context. Use --save-cluster-auth=false to deploy without storing credentials. ($FUNC_SAVE_CLUSTER_AUTH) (default true) --service-account string Service account to be used in the deployed function ($FUNC_SERVICE_ACCOUNT) --token string Token to use when pushing to the registry. ($FUNC_TOKEN) --username string Username to use when pushing to the registry. ($FUNC_USERNAME) diff --git a/docs/reference/func_describe.md b/docs/reference/func_describe.md index 7e5254392b..1904a9ff7e 100644 --- a/docs/reference/func_describe.md +++ b/docs/reference/func_describe.md @@ -29,11 +29,13 @@ func describe --output yaml --path myotherfunc ### Options ``` - -h, --help help for describe - -n, --namespace string The namespace in which to look for the named function. ($FUNC_NAMESPACE) (default "default") - -o, --output string Output format (human|plain|json|yaml|url) ($FUNC_OUTPUT) (default "human") - -p, --path string Path to the function. Default is current directory ($FUNC_PATH) - -v, --verbose Print verbose logs ($FUNC_VERBOSE) + --cluster string Specify a cluster api url for your function deployment. ($FUNC_CLUSTER) + --cluster-token string Bearer token for cluster authentication. ($FUNC_CLUSTER_TOKEN) + -h, --help help for describe + -n, --namespace string The namespace in which to look for the named function. ($FUNC_NAMESPACE) (default "default") + -o, --output string Output format (human|plain|json|yaml|url) ($FUNC_OUTPUT) (default "human") + -p, --path string Path to the function. Default is current directory ($FUNC_PATH) + -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` ### SEE ALSO diff --git a/docs/reference/func_invoke.md b/docs/reference/func_invoke.md index 21cf277cdb..90bb76fbbf 100644 --- a/docs/reference/func_invoke.md +++ b/docs/reference/func_invoke.md @@ -96,21 +96,23 @@ func invoke ### Options ``` - -c, --confirm Prompt to confirm options interactively ($FUNC_CONFIRM) - --content-type string Content Type of the data. ($FUNC_CONTENT_TYPE) (default "application/json") - --data string Data to send in the request. ($FUNC_DATA) (default "{\"message\":\"Hello World\"}") - -e, --extension strings Extensions as key=value pairs. Can be repeated. cloudevents only ($FUNC_EXTENSION) - --file string Path to a file to use as data. Overrides --data flag and should be sent with a correct --content-type. ($FUNC_FILE) - -f, --format string Format of message to send, 'http' or 'cloudevent(s)'. Default is to choose automatically. ($FUNC_FORMAT) - -h, --help help for invoke - --id string ID for the request data. ($FUNC_ID) - -i, --insecure Allow insecure server connections when using SSL. ($FUNC_INSECURE) - -p, --path string Path to the function. Default is current directory ($FUNC_PATH) - --request-type string Type of request to use. Can be POST or GET. ($FUNC_REQUEST_TYPE) (default "POST") - --source string Source value for the request data. ($FUNC_SOURCE) (default "/boson/fn") - -t, --target string Function instance to invoke. Can be 'local', 'remote' or a URL. Defaults to auto-discovery if not provided. ($FUNC_TARGET) - --type string Type value for the request data. ($FUNC_TYPE) (default "boson.fn") - -v, --verbose Print verbose logs ($FUNC_VERBOSE) + --cluster string Specify a cluster api url. ($FUNC_CLUSTER) + --cluster-token string Bearer token for cluster authentication. ($FUNC_CLUSTER_TOKEN) + -c, --confirm Prompt to confirm options interactively ($FUNC_CONFIRM) + --content-type string Content Type of the data. ($FUNC_CONTENT_TYPE) (default "application/json") + --data string Data to send in the request. ($FUNC_DATA) (default "{\"message\":\"Hello World\"}") + -e, --extension strings Extensions as key=value pairs. Can be repeated. cloudevents only ($FUNC_EXTENSION) + --file string Path to a file to use as data. Overrides --data flag and should be sent with a correct --content-type. ($FUNC_FILE) + -f, --format string Format of message to send, 'http' or 'cloudevent(s)'. Default is to choose automatically. ($FUNC_FORMAT) + -h, --help help for invoke + --id string ID for the request data. ($FUNC_ID) + -i, --insecure Allow insecure server connections when using SSL. ($FUNC_INSECURE) + -p, --path string Path to the function. Default is current directory ($FUNC_PATH) + --request-type string Type of request to use. Can be POST or GET. ($FUNC_REQUEST_TYPE) (default "POST") + --source string Source value for the request data. ($FUNC_SOURCE) (default "/boson/fn") + -t, --target string Function instance to invoke. Can be 'local', 'remote' or a URL. Defaults to auto-discovery if not provided. ($FUNC_TARGET) + --type string Type value for the request data. ($FUNC_TYPE) (default "boson.fn") + -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` ### SEE ALSO diff --git a/docs/reference/func_list.md b/docs/reference/func_list.md index f743e66334..5bd65caf0a 100644 --- a/docs/reference/func_list.md +++ b/docs/reference/func_list.md @@ -31,11 +31,13 @@ func list --all-namespaces --output json ### Options ``` - -A, --all-namespaces List functions in all namespaces. If set, the --namespace flag is ignored. - -h, --help help for list - -n, --namespace string The namespace for which to list functions. ($FUNC_NAMESPACE) (default "default") - -o, --output string Output format (human|plain|json|xml|yaml) ($FUNC_OUTPUT) (default "human") - -v, --verbose Print verbose logs ($FUNC_VERBOSE) + -A, --all-namespaces List functions in all namespaces. If set, the --namespace flag is ignored. + --cluster string Specify a cluster api url. ($FUNC_CLUSTER) + --cluster-token string Bearer token for cluster authentication. ($FUNC_CLUSTER_TOKEN) + -h, --help help for list + -n, --namespace string The namespace for which to list functions. ($FUNC_NAMESPACE) (default "default") + -o, --output string Output format (human|plain|json|xml|yaml) ($FUNC_OUTPUT) (default "human") + -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` ### SEE ALSO diff --git a/docs/reference/func_logs.md b/docs/reference/func_logs.md index 8423776f5f..df59c42fcd 100644 --- a/docs/reference/func_logs.md +++ b/docs/reference/func_logs.md @@ -35,12 +35,14 @@ func logs --since 5m ### Options ``` - -h, --help help for logs - --name string Name of the function to get logs from ($FUNC_NAME) - -n, --namespace string The namespace of the function ($FUNC_NAMESPACE) (default "default") - -p, --path string Path to the function. Default is current directory ($FUNC_PATH) - --since string Return logs newer than a relative duration like 5s, 2m, or 3h ($FUNC_LOGS_SINCE) (default "1m") - -v, --verbose Print verbose logs ($FUNC_VERBOSE) + --cluster string Specify a cluster api url. ($FUNC_CLUSTER) + --cluster-token string Bearer token for cluster authentication. ($FUNC_CLUSTER_TOKEN) + -h, --help help for logs + --name string Name of the function to get logs from ($FUNC_NAME) + -n, --namespace string The namespace of the function ($FUNC_NAMESPACE) (default "default") + -p, --path string Path to the function. Default is current directory ($FUNC_PATH) + --since string Return logs newer than a relative duration like 5s, 2m, or 3h ($FUNC_LOGS_SINCE) (default "1m") + -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` ### SEE ALSO diff --git a/e2e/e2e_core_test.go b/e2e/e2e_core_test.go index d2bae71d13..3603a8936d 100644 --- a/e2e/e2e_core_test.go +++ b/e2e/e2e_core_test.go @@ -12,6 +12,7 @@ import ( "testing" fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" "knative.dev/func/pkg/knative" ) @@ -398,7 +399,9 @@ func TestCore_Delete(t *testing.T) { } // Check it appears in the list - client := fn.New(fn.WithListers(knative.NewLister(false))) + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + client := fn.New(fn.WithListers(knative.NewLister(kc, false))) list, err := client.List(t.Context(), Namespace) if err != nil { t.Fatal(err) @@ -425,3 +428,92 @@ func TestCore_Delete(t *testing.T) { t.Fatalf("Instance %q is still shown as available", name) } } + +// TestCore_DeployWithCachedAuth verifies that after a successful deploy using +// kubeconfig, the cluster auth is extracted and persisted to local.yaml. +// A subsequent deploy with an invalid kubeconfig should still succeed because +// the stored auth is used via ConfigOverrides. +func TestCore_DeployWithCachedAuth(t *testing.T) { + name := "func-e2e-test-cached-auth" + root := fromCleanEnv(t, name) + + // Step 1: Init and deploy using normal kubeconfig + if err := newCmd(t, "init", "-l=go").Run(); err != nil { + t.Fatal(err) + } + if err := newCmd(t, "deploy").Run(); err != nil { + t.Fatal(err) + } + defer func() { + // Restore valid kubeconfig for cleanup + os.Setenv("KUBECONFIG", Kubeconfig) + clean(t, name, Namespace) + }() + + if !waitFor(t, ksvcUrl(name)) { + t.Fatal("function did not deploy correctly on first deploy") + } + + // Step 2: Verify local.yaml has auth cached + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if f.Deploy.Cluster == "" { + t.Fatal("expected Deploy.Cluster to be set after first deploy") + } + if f.Local.FindAuth(f.Deploy.Cluster) == nil { + t.Fatalf("expected auth entry for cluster %q in local.yaml after first deploy", f.Deploy.Cluster) + } + + // Step 3: Point kubeconfig to /dev/null (invalid) and redeploy + // The deploy should succeed because stored auth from local.yaml is used + // via ConfigOverrides on top of the (broken) kubeconfig. + os.Setenv("KUBECONFIG", "/dev/null") + + if err := newCmd(t, "deploy", "--build=false", "--push=false").Run(); err != nil { + t.Fatalf("expected redeploy with cached auth to succeed, got: %v", err) + } + + // Step 4: the pinned cluster and cached auth survive the broken-kubeconfig + // redeploy (the target did not silently follow the active kubeconfig). + if f, err = fn.NewFunction(root); err != nil { + t.Fatal(err) + } + if f.Deploy.Cluster == "" { + t.Fatal("expected Deploy.Cluster to remain pinned after redeploy with cached auth") + } + if f.Local.FindAuth(f.Deploy.Cluster) == nil { + t.Fatalf("expected cached auth for %q to persist after redeploy", f.Deploy.Cluster) + } +} + +// TestCore_DeploySaveAuthFalse verifies that `deploy --save-cluster-auth=false` still +// deploys via the active kubeconfig (the "old way") and pins the cluster, but +// does NOT cache credentials into .func/local.yaml. +func TestCore_DeploySaveAuthFalse(t *testing.T) { + name := "func-e2e-test-save-auth-false" + root := fromCleanEnv(t, name) + defer clean(t, name, Namespace) + + if err := newCmd(t, "init", "-l=go").Run(); err != nil { + t.Fatal(err) + } + if err := newCmd(t, "deploy", "--save-cluster-auth=false").Run(); err != nil { + t.Fatal(err) + } + if !waitFor(t, ksvcUrl(name)) { + t.Fatal("function did not deploy with --save-cluster-auth=false") + } + + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if f.Deploy.Cluster == "" { + t.Fatal("expected Deploy.Cluster to be pinned even with --save-cluster-auth=false") + } + if len(f.Local.Auth) != 0 { + t.Fatalf("expected no cached credentials with --save-cluster-auth=false, got %d entries", len(f.Local.Auth)) + } +} diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index abbb2511e5..1525498b3e 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -29,7 +29,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" "knative.dev/func/pkg/k8s" ) @@ -859,11 +858,9 @@ 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() - if err != nil { - t.Fatal(err) - } - clientset, err := kubernetes.NewForConfig(config) + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + clientset, err := kc.Clientset() if err != nil { t.Fatal(err) } @@ -882,11 +879,9 @@ 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() - if err != nil { - t.Fatal(err) - } - clientset, err := kubernetes.NewForConfig(config) + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + clientset, err := kc.Clientset() if err != nil { t.Fatal(err) } diff --git a/pkg/builders/builders_int_test.go b/pkg/builders/builders_int_test.go index a57fbd58c0..0d425a4198 100644 --- a/pkg/builders/builders_int_test.go +++ b/pkg/builders/builders_int_test.go @@ -374,7 +374,8 @@ func servePrivateGit(ctx context.Context, t *testing.T, certDir string) { image = "ghcr.io/matejvasek/git-private:latest" ) - k8sClient, err := k8s.NewKubernetesClientset() + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + k8sClient, err := k8s.NewClient(cc).Clientset() if err != nil { t.Fatal(err) } diff --git a/pkg/config/config.go b/pkg/config/config.go index 0b8dddd9e7..5ad1e35d0a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -12,7 +12,6 @@ import ( "gopkg.in/yaml.v2" "knative.dev/func/pkg/builders" fn "knative.dev/func/pkg/functions" - "knative.dev/func/pkg/k8s" ) const ( @@ -31,17 +30,18 @@ const ( // Global configuration settings. type Global struct { - Builder string `yaml:"builder,omitempty"` - Confirm bool `yaml:"confirm,omitempty"` - Language string `yaml:"language,omitempty"` - Namespace string `yaml:"namespace,omitempty"` - Registry string `yaml:"registry,omitempty"` - Verbose bool `yaml:"verbose,omitempty"` + Builder string `yaml:"builder,omitempty"` + Confirm bool `yaml:"confirm,omitempty"` + Language string `yaml:"language,omitempty"` + Namespace string `yaml:"namespace,omitempty"` + Registry string `yaml:"registry,omitempty"` + Verbose bool `yaml:"verbose,omitempty"` + RegistryInsecure bool `yaml:"registryInsecure,omitempty"` + Cluster string `yaml:"cluster,omitempty"` + // NOTE: all members must include their yaml serialized names, even when // this is the default, because these tag values are used for the static // getter/setter accessors to match requests. - - RegistryInsecure bool `yaml:"registryInsecure,omitempty"` } // New Config struct with all members set to static defaults. See NewDefaults @@ -54,22 +54,6 @@ func New() Global { } } -// RegistyDefault 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 { - // 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 "" - } -} - // NewDefault returns a config populated by global defaults as defined by the // config file located in .Path() (the global func settings path, which is // @@ -141,6 +125,9 @@ func (c Global) Apply(f fn.Function) Global { // Unconditional because bool has no "empty value". Works because // viper resolves the correct precedence via our defaulting. c.RegistryInsecure = f.RegistryInsecure + if f.Deploy.Cluster != "" { + c.Cluster = f.Deploy.Cluster + } return c } @@ -164,6 +151,9 @@ func (c Global) Configure(f fn.Function) fn.Function { // viper resolves the correct precedence via our defaulting. f.RegistryInsecure = c.RegistryInsecure + // Unconditional to allow --cluster= (empty value) to use kubeconfig context + f.Deploy.Cluster = c.Cluster + return f } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 1e3dd287b8..d8a0d6f655 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -372,6 +372,7 @@ func TestList(t *testing.T) { values := config.List() expected := []string{ "builder", + "cluster", "confirm", "language", "namespace", diff --git a/pkg/deployer/testing/integration_test_helper.go b/pkg/deployer/testing/integration_test_helper.go index 379cad6a73..453822b42c 100644 --- a/pkg/deployer/testing/integration_test_helper.go +++ b/pkg/deployer/testing/integration_test_helper.go @@ -34,6 +34,11 @@ import ( "knative.dev/func/pkg/k8s" ) +func defaultK8sClient() *k8s.Client { + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + return k8s.NewClient(cc) +} + // TestInt_Deploy ensures that the deployer creates a callable service. // See TestInt_Metadata for Labels, Volumes, Envs. // See TestInt_Events for Subscriptions @@ -458,7 +463,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 := defaultK8sClient().Clientset() if err != nil { t.Fatal(err) } @@ -608,7 +613,7 @@ func TestInt_EnvsUpdate(t *testing.T, deployer fn.Deployer, remover fn.Remover, t.Fatal(err) } - cliSet, err := k8s.NewKubernetesClientset() + cliSet, err := defaultK8sClient().Clientset() if err != nil { t.Fatal(err) } @@ -663,7 +668,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 := defaultK8sClient().Clientset() if err != nil { t.Fatal(err) } @@ -759,7 +764,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, namespace, selector, "user-container", "", &now, buff) + _ = k8s.GetPodLogsBySelector(ctx, defaultK8sClient(), namespace, selector, "user-container", "", &now, buff) }() depRes, err := deployer.Deploy(ctx, function) @@ -839,7 +844,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, namespace, selector, "user-container", "", &now, redeployLogBuff) + _ = k8s.GetPodLogsBySelector(ctx, defaultK8sClient(), namespace, selector, "user-container", "", &now, redeployLogBuff) }() _, err = deployer.Deploy(ctx, function) @@ -1040,7 +1045,7 @@ func createTrigger(t *testing.T, ctx context.Context, namespace, triggerName str }, }, } - eventingClient, err := knative.NewEventingClient(namespace) + eventingClient, err := knative.NewEventingClient(defaultK8sClient(), namespace) if err != nil { t.Fatal(err) } @@ -1073,7 +1078,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 := defaultK8sClient().Clientset() if err != nil { t.Fatal(err) } @@ -1104,7 +1109,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 := defaultK8sClient().Clientset() if err != nil { t.Fatal(err) } @@ -1131,19 +1136,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 := defaultK8sClient().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 := defaultK8sClient().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(defaultK8sClient(), namespace); err == nil { _ = eventingClient.DeleteTrigger(context.Background(), name) } }) @@ -1224,7 +1229,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(defaultK8sClient())), ) f, err := client.Init(fn.Function{ @@ -1270,7 +1275,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 := defaultK8sClient().ClientConfig() if err != nil { t.Fatalf("getting kubernetes config: %v", err) } @@ -1394,8 +1399,8 @@ 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) + kc := defaultK8sClient() + dialer, err := k8s.NewInClusterDialer(ctx, kc) if err != nil { return nil, noopDeferFunc, fmt.Errorf("failed to create in-cluster dialer: %w", err) } diff --git a/pkg/functions/client.go b/pkg/functions/client.go index a7f45c4254..42b4e9a2e4 100644 --- a/pkg/functions/client.go +++ b/pkg/functions/client.go @@ -861,7 +861,6 @@ func (c *Client) Deploy(ctx context.Context, f Function, oo ...DeployOption) (Fu return f, ErrNameRequired } - // Warn if moving changingNamespace := func(f Function) bool { // We're changing namespace if: return f.Deploy.Namespace != "" && // it's already deployed @@ -873,7 +872,7 @@ func (c *Client) Deploy(ctx context.Context, f Function, oo ...DeployOption) (Fu // On forced namespace change (using --namespace flag) if changingNamespace(f) { if c.verbose { - fmt.Fprintf(os.Stderr, "Moving Function from %q to %q \n", f.Deploy.Namespace, f.Namespace) + fmt.Fprintf(os.Stderr, "Moving Function from namespace %q to %q\n", f.Deploy.Namespace, f.Namespace) } // c.Remove removes a Function in f.Deploy.Namespace which removes the OLD Function diff --git a/pkg/functions/client_int_test.go b/pkg/functions/client_int_test.go index b474e57621..6721a2e5ac 100644 --- a/pkg/functions/client_int_test.go +++ b/pkg/functions/client_int_test.go @@ -212,7 +212,9 @@ func TestInt_Update_WithAnnotationsAndLabels(t *testing.T) { functionName := "updateannlab" verbose := false - servingClient, err := knative.NewServingClient(DefaultIntTestNamespace) + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + servingClient, err := knative.NewServingClient(kc, DefaultIntTestNamespace) if err != nil { t.Fatal(err) } @@ -666,22 +668,26 @@ func resetEnv() { // newClient creates an instance of the func client with concrete impls // sufficient for running integration tests. func newClient(verbose bool) *fn.Client { + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) 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.WithListers(knative.NewLister(verbose), k8s.NewLister(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), ) } // copy of newClient just with s2i methods instead func newClientWithS2i(verbose bool) *fn.Client { + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) return fn.New( fn.WithRegistry(DefaultIntTestRegistry), fn.WithRegistryInsecure(true), @@ -689,10 +695,10 @@ 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.WithListers(knative.NewLister(verbose), k8s.NewLister(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/functions/function.go b/pkg/functions/function.go index ff07f41225..e4a245761f 100644 --- a/pkg/functions/function.go +++ b/pkg/functions/function.go @@ -48,6 +48,66 @@ type Local struct { // Remote indicates the deployment (and possibly build) process are to // be triggered in a remote environment rather than run locally. Remote bool `yaml:"remote,omitempty"` + + // Auth holds per-cluster authentication entries + Auth []AuthEntry `yaml:"auth,omitempty"` +} + +// AuthEntry is the complete structure for verification on cluster side and +// authentication on user side for cluster to deploy to. +type AuthEntry struct { + ClusterURL string `yaml:"cluster-url"` + Cluster ClusterTLS `yaml:"cluster,omitempty"` + User UserAuth `yaml:"user,omitempty"` +} + +type ClusterTLS struct { + CertificateAuthorityData string `yaml:"certificate-authority-data,omitempty"` + CertificateAuthority string `yaml:"certificate-authority,omitempty"` + InsecureSkipTLSVerify bool `yaml:"insecure-skip-tls-verify,omitempty"` +} + +// UserAuth holds user credentials for authenticating to a cluster. +type UserAuth struct { + ClientCertificateData string `yaml:"client-certificate-data,omitempty"` + ClientKeyData string `yaml:"client-key-data,omitempty"` + ClientCertificate string `yaml:"client-certificate,omitempty"` + ClientKey string `yaml:"client-key,omitempty"` + Token string `yaml:"token,omitempty"` +} + +// FindAuth returns first AuthEntry matching the cluster URL it finds, or nil +// if no entry matches. +func (l Local) FindAuth(url string) *AuthEntry { + url = normalizeURL(url) + for i := range l.Auth { + if normalizeURL(l.Auth[i].ClusterURL) == url { + return &l.Auth[i] + } + } + return nil +} + +// SetAuth upserts an auth entry for the given cluster URL. If an entry with the +// same URL already exists it is updated; otherwise a new entry is appended. +func (l *Local) SetAuth(clusterURL string, cluster ClusterTLS, user UserAuth) { + clusterURL = normalizeURL(clusterURL) + for i := range l.Auth { + if normalizeURL(l.Auth[i].ClusterURL) == clusterURL { + l.Auth[i].Cluster = cluster + l.Auth[i].User = user + return + } + } + l.Auth = append(l.Auth, AuthEntry{ + ClusterURL: clusterURL, + Cluster: cluster, + User: user, + }) +} + +func normalizeURL(url string) string { + return strings.TrimRight(url, "/") } // Function @@ -196,6 +256,9 @@ type DeploySpec struct { // Namespace into which the function was deployed on supported platforms. Namespace string `yaml:"namespace,omitempty"` + // Cluster is the cluster server api URL where the function is deployed + Cluster string `yaml:"cluster,omitempty"` + // Image is the deployed image including sha256 Image string `yaml:"image,omitempty"` @@ -503,7 +566,14 @@ func (f Function) Write() (err error) { } localConfigPath := filepath.Join(f.Root, RunDataDir, RunDataLocalFile) - if err = os.WriteFile(localConfigPath, bb, 0644); err != nil { + if err = os.WriteFile(localConfigPath, bb, 0600); err != nil { + return + } + // os.WriteFile does not tighten the mode of a pre-existing file, so enforce + // 0600 on every write. + // gauron99: for more security we could stat file -> chmod -> write instead + // or move the auth to a separate file (since remote flag is bundled up here) + if err = os.Chmod(localConfigPath, 0600); err != nil { return } diff --git a/pkg/functions/function_unit_test.go b/pkg/functions/function_unit_test.go index 60335c3a99..80f57ed76a 100644 --- a/pkg/functions/function_unit_test.go +++ b/pkg/functions/function_unit_test.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "reflect" + "runtime" "strings" "testing" @@ -358,3 +359,196 @@ func Test_WarnIfLegacyS2IScaffolding(t *testing.T) { }) } } + +// TestLocalAuth_SetFindPersist verifies that auth entries stored via SetAuth +// are persisted to .func/local.yaml and survive a function reload. +func TestLocalAuth_SetFindPersist(t *testing.T) { + root := FromTempDirectory(t) + + // Create and initialize a function + f := fn.Function{Runtime: "go", Root: root} + f, err := fn.New().Init(f) + if err != nil { + t.Fatal(err) + } + + // Store auth for a cluster + f.Local.SetAuth("https://cluster.example.com:6443", fn.ClusterTLS{ + CertificateAuthorityData: "Y2EtZGF0YQ==", + InsecureSkipTLSVerify: false, + }, fn.UserAuth{ + Token: "my-token", + ClientCertificateData: "Y2VydC1kYXRh", + ClientKeyData: "a2V5LWRhdGE=", + }) + + // Write to disk + if err := f.Write(); err != nil { + t.Fatal(err) + } + + // Verify local.yaml exists + localPath := filepath.Join(root, ".func", "local.yaml") + if _, err := os.Stat(localPath); err != nil { + t.Fatalf("expected local.yaml to exist: %v", err) + } + + // Reload function from disk + f2, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + // FindAuth should return the stored entry + entry := f2.Local.FindAuth("https://cluster.example.com:6443") + if entry == nil { + t.Fatal("expected auth entry after reload, got nil") + } + if entry.User.Token != "my-token" { + t.Fatalf("expected token 'my-token', got %q", entry.User.Token) + } + if entry.Cluster.CertificateAuthorityData != "Y2EtZGF0YQ==" { + t.Fatalf("expected CA data preserved, got %q", entry.Cluster.CertificateAuthorityData) + } + if entry.User.ClientCertificateData != "Y2VydC1kYXRh" { + t.Fatalf("expected cert data preserved, got %q", entry.User.ClientCertificateData) + } + if entry.User.ClientKeyData != "a2V5LWRhdGE=" { + t.Fatalf("expected key data preserved, got %q", entry.User.ClientKeyData) + } +} + +// TestLocalAuth_FindMiss verifies FindAuth returns nil for unknown cluster URLs. +func TestLocalAuth_FindMiss(t *testing.T) { + local := fn.Local{} + local.SetAuth("https://known.example.com", fn.ClusterTLS{}, fn.UserAuth{Token: "tok"}) + + if entry := local.FindAuth("https://unknown.example.com"); entry != nil { + t.Fatalf("expected nil for unknown URL, got %v", entry) + } +} + +// TestLocalAuth_Upsert verifies SetAuth updates existing entries. +func TestLocalAuth_Upsert(t *testing.T) { + local := fn.Local{} + local.SetAuth("https://cluster.example.com", fn.ClusterTLS{}, fn.UserAuth{Token: "old-token"}) + local.SetAuth("https://cluster.example.com", fn.ClusterTLS{}, fn.UserAuth{Token: "new-token"}) + + if len(local.Auth) != 1 { + t.Fatalf("expected 1 entry after upsert, got %d", len(local.Auth)) + } + if local.Auth[0].User.Token != "new-token" { + t.Fatalf("expected upserted token, got %q", local.Auth[0].User.Token) + } +} + +// TestLocalAuth_MultipleEntries verifies multiple clusters can be stored. +func TestLocalAuth_MultipleEntries(t *testing.T) { + local := fn.Local{} + local.SetAuth("https://cluster-a.example.com", fn.ClusterTLS{}, fn.UserAuth{Token: "tok-a"}) + local.SetAuth("https://cluster-b.example.com", fn.ClusterTLS{}, fn.UserAuth{Token: "tok-b"}) + + if len(local.Auth) != 2 { + t.Fatalf("expected 2 entries, got %d", len(local.Auth)) + } + a := local.FindAuth("https://cluster-a.example.com") + b := local.FindAuth("https://cluster-b.example.com") + if a == nil || a.User.Token != "tok-a" { + t.Fatal("cluster-a auth not found or wrong") + } + if b == nil || b.User.Token != "tok-b" { + t.Fatal("cluster-b auth not found or wrong") + } +} + +// TestLocalAuth_LocalYAMLMode0600 ensures the credential file is written 0600. +func TestLocalAuth_LocalYAMLMode0600(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("file mode bits are not meaningful on Windows") + } + root := FromTempDirectory(t) + f, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}) + if err != nil { + t.Fatal(err) + } + f.Local.SetAuth("https://cluster.example.com:6443", fn.ClusterTLS{}, fn.UserAuth{Token: "secret"}) + if err := f.Write(); err != nil { + t.Fatal(err) + } + info, err := os.Stat(filepath.Join(root, ".func", "local.yaml")) + if err != nil { + t.Fatal(err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Fatalf("local.yaml mode = %o, want 0600", perm) + } +} + +// TestLocalAuth_TightensPreexisting0644 ensures a pre-existing, loosened +// local.yaml is re-tightened to 0600 on every Write -- unconditionally, even +// when the rewrite carries no credentials (os.WriteFile does not change the +// mode of an already-existing file). local.yaml is private machine-local +// state and must never be left group/world-readable. +func TestLocalAuth_TightensPreexisting0644(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("file mode bits are not meaningful on Windows") + } + root := FromTempDirectory(t) + f, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}) + if err != nil { + t.Fatal(err) + } + // Write a local.yaml, then loosen it as an older func (or a stray umask) + // would have. + if err := f.Write(); err != nil { + t.Fatal(err) + } + localPath := filepath.Join(root, ".func", "local.yaml") + if err := os.Chmod(localPath, 0o644); err != nil { + t.Fatal(err) + } + // Reload and write again WITHOUT any credentials -> must still be + // re-tightened to 0600. The invariant holds regardless of auth content. + // (A non-auth change is needed to force a write, since Write() no-ops when + // the function is unmodified.) + if f, err = fn.NewFunction(root); err != nil { + t.Fatal(err) + } + f.Local.Remote = true + if err := f.Write(); err != nil { + t.Fatal(err) + } + info, err := os.Stat(localPath) + if err != nil { + t.Fatal(err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Fatalf("pre-existing local.yaml not tightened: mode = %o, want 0600", perm) + } +} + +// TestLocalAuth_NotInFuncYAML ensures credentials never leak into the +// source-controlled func.yaml, while the (non-secret) cluster URL is recorded. +func TestLocalAuth_NotInFuncYAML(t *testing.T) { + root := FromTempDirectory(t) + f := fn.Function{Runtime: "go", Root: root, + Deploy: fn.DeploySpec{Cluster: "https://cluster.example.com:6443"}} + f, err := fn.New().Init(f) + if err != nil { + t.Fatal(err) + } + f.Local.SetAuth("https://cluster.example.com:6443", fn.ClusterTLS{}, fn.UserAuth{Token: "super-secret-token"}) + if err := f.Write(); err != nil { + t.Fatal(err) + } + raw, err := os.ReadFile(filepath.Join(root, "func.yaml")) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(raw), "super-secret-token") { + t.Fatalf("func.yaml leaked the stored token:\n%s", raw) + } + if !strings.Contains(string(raw), "https://cluster.example.com:6443") { + t.Fatalf("func.yaml should record the (non-secret) cluster URL:\n%s", raw) + } +} diff --git a/pkg/http/openshift.go b/pkg/http/openshift.go index 0d9b5d8939..410b385167 100644 --- a/pkg/http/openshift.go +++ b/pkg/http/openshift.go @@ -13,15 +13,15 @@ import ( const openShiftRegistryHost = "image-registry.openshift-image-registry.svc" // WithOpenShiftServiceCA enables trust to OpenShift's service CA for internal image registry -func WithOpenShiftServiceCA() Option { +func WithOpenShiftServiceCA(c *k8s.Client) Option { var err error var ca *x509.Certificate var o sync.Once selectCA := func(ctx context.Context, serverName string) (*x509.Certificate, error) { - if strings.HasPrefix(serverName, openShiftRegistryHost) { + if c != nil && strings.HasPrefix(serverName, openShiftRegistryHost) { o.Do(func() { - ca, err = k8s.GetOpenShiftServiceCA(ctx) + ca, err = c.GetOpenShiftServiceCA(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..2ba0afd0f5 100644 --- a/pkg/http/openshift_int_test.go +++ b/pkg/http/openshift_int_test.go @@ -6,17 +6,20 @@ import ( "net/http" "testing" + fn "knative.dev/func/pkg/functions" fnhttp "knative.dev/func/pkg/http" "knative.dev/func/pkg/k8s" ) func TestInt_RoundTripper(t *testing.T) { - if !k8s.IsOpenShift() { + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + if !kc.IsOpenshift() { 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..df18a2fbaa 100644 --- a/pkg/http/transport.go +++ b/pkg/http/transport.go @@ -54,11 +54,13 @@ func WithInsecureSkipVerify(insecureSkipVerify bool) Option { // 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 { +func NewRoundTripper(kc *k8s.Client, opts ...Option) RoundTripCloser { o := options{ - inClusterDialer: k8s.NewLazyInitInClusterDialer(k8s.GetClientConfig()), insecureSkipVerify: false, } + if kc != nil { + o.inClusterDialer = k8s.NewLazyInitInClusterDialer(kc) + } for _, option := range opts { option(&o) } @@ -133,6 +135,9 @@ func (d *dialerWithFallback) DialContext(ctx context.Context, network, address s return nil, err } + if d.fallbackDialer == nil { + return nil, err + } return d.fallbackDialer.DialContext(ctx, network, address) } @@ -145,7 +150,9 @@ func (d *dialerWithFallback) Close() error { errs = append(errs, err) } - err = d.fallbackDialer.Close() + if d.fallbackDialer != nil { + err = d.fallbackDialer.Close() + } if err != nil { errs = append(errs, err) } diff --git a/pkg/http/transport_test.go b/pkg/http/transport_test.go index ee6b9e744f..8d02db76b4 100644 --- a/pkg/http/transport_test.go +++ b/pkg/http/transport_test.go @@ -39,7 +39,7 @@ func TestCustomCA(t *testing.T) { backingAddr: inClusterAddr, } - tr := fnhttp.NewRoundTripper( + tr := fnhttp.NewRoundTripper(nil, fnhttp.WithSelectCA(mockSelectCA), fnhttp.WithInClusterDialer(mockInCusterDialer)) defer tr.Close() diff --git a/pkg/k8s/client.go b/pkg/k8s/client.go index 8d528280df..fd050b506d 100644 --- a/pkg/k8s/client.go +++ b/pkg/k8s/client.go @@ -1,13 +1,21 @@ package k8s import ( + "encoding/base64" "fmt" + "os" + "strings" + "sync" "time" "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" + + fn "knative.dev/func/pkg/functions" ) const ( @@ -15,45 +23,241 @@ const ( DefaultErrorWindowTimeout = 2 * time.Second ) -func NewClientAndResolvedNamespace(ns string) (*kubernetes.Clientset, string, error) { +// Client wraps a clientcmd.ClientConfig and provides convenience methods +// for creating Kubernetes clients. All cluster access in the codebase should +// go through a Client instance rather than calling package-level functions. +type Client struct { + cc clientcmd.ClientConfig + openshift bool + openShiftOnce sync.Once +} + +// NewClient creates a Client from the given ClientConfig. +func NewClient(cc clientcmd.ClientConfig) *Client { + return &Client{cc: cc} +} + +// NewClientWithOpenShift creates a Client with a pre-set OpenShift detection +// result. For testing only. +func NewClientWithOpenShift(cc clientcmd.ClientConfig, isOpenShift bool) *Client { + c := &Client{cc: cc, openshift: isOpenShift} + c.openShiftOnce.Do(func() {}) // prevent real detection + return c +} + +// ClientConfig returns the underlying rest.Config. +func (c *Client) ClientConfig() (*rest.Config, error) { + return c.cc.ClientConfig() +} + +// Clientset creates a new kubernetes.Clientset. +func (c *Client) Clientset() (*kubernetes.Clientset, error) { + cfg, err := c.cc.ClientConfig() + if err != nil { + return nil, fmt.Errorf("failed to create new kubernetes client: %w", err) + } + return kubernetes.NewForConfig(cfg) +} + +// ClientAndNamespace creates a Clientset and resolves the namespace, +// falling back to the default namespace from the config if ns is empty. +func (c *Client) ClientAndNamespace(ns string) (*kubernetes.Clientset, string, error) { var err error if ns == "" { - ns, err = GetDefaultNamespace() + ns, err = c.DefaultNamespace() if err != nil { return nil, ns, err } } - - client, err := NewKubernetesClientset() + client, err := c.Clientset() return client, ns, err } -func NewKubernetesClientset() (*kubernetes.Clientset, error) { - restConfig, err := GetClientConfig().ClientConfig() +// DynamicClient creates a new dynamic.Interface. +func (c *Client) DynamicClient() (dynamic.Interface, error) { + cfg, err := c.cc.ClientConfig() if err != nil { return nil, fmt.Errorf("failed to create new kubernetes client: %w", err) } + return dynamic.NewForConfig(cfg) +} + +// DefaultNamespace returns the default namespace from the config. +func (c *Client) DefaultNamespace() (string, error) { + ns, _, err := c.cc.Namespace() + return ns, err +} - return kubernetes.NewForConfig(restConfig) +// RawConfig returns the raw kubeconfig API config. +func (c *Client) RawConfig() (clientcmdapi.Config, error) { + return c.cc.RawConfig() } -func NewDynamicClient() (dynamic.Interface, error) { - restConfig, err := GetClientConfig().ClientConfig() +// Auth exports the already-resolved rest.Config as fn types suitable for +// storage in local.yaml. After a successful deploy the client already holds +// the merged result of kubeconfig + overrides, so there is no need to +// re-parse the raw kubeconfig. +func (c *Client) Auth() (clusterURL string, cluster fn.ClusterTLS, user fn.UserAuth, err error) { + cfg, err := c.cc.ClientConfig() if err != nil { - return nil, fmt.Errorf("failed to create new kubernetes client: %w", err) + return } - return dynamic.NewForConfig(restConfig) -} + clusterURL = cfg.Host + + if len(cfg.CAData) > 0 { + cluster.CertificateAuthorityData = base64.StdEncoding.EncodeToString(cfg.CAData) + } + cluster.CertificateAuthority = cfg.CAFile + cluster.InsecureSkipTLSVerify = cfg.Insecure + + user.Token = cfg.BearerToken + if len(cfg.CertData) > 0 { + user.ClientCertificateData = base64.StdEncoding.EncodeToString(cfg.CertData) + } + if len(cfg.KeyData) > 0 { + user.ClientKeyData = base64.StdEncoding.EncodeToString(cfg.KeyData) + } + user.ClientCertificate = cfg.CertFile + user.ClientKey = cfg.KeyFile -// GetDefaultNamespace returns default namespace -func GetDefaultNamespace() (namespace string, err error) { - namespace, _, err = GetClientConfig().Namespace() return } -func GetClientConfig() clientcmd.ClientConfig { +// BuildClientConfig creates the k8s client config with possible overrides to +// the cluster url or its auth fields. Base64-encoded string fields from +// local.yaml are decoded to []byte for the clientcmd API. +// +// When a cluster URL is provided but no stored auth exists for it, the +// kubeconfig is searched for a context whose cluster matches the URL. +// If the active context matches, its auth is used. Otherwise exactly one +// matching context is required; zero or multiple matches produce an error. +func BuildClientConfig(url, token, namespace string, local fn.Local) (clientcmd.ClientConfig, error) { + // For proper identification we need 3 concrete items in order to not rely + // on the kubeconfig active context + + co := clientcmd.ConfigOverrides{} + + // if cluster url is detected + if url != "" { + // 1) authentication, certificates, keys (or token further below) + if entry := local.FindAuth(url); entry != nil { + co = authEntryOverrides(entry) // add overrides from local.yaml + } else if token == "" { + resolved, err := resolveKubeconfigAuth(url) + if err != nil { + return nil, err + } + co = resolved // use kubeconfig context (active or exactly 1 match) + } + // 2) cluster url + co.ClusterInfo.Server = url + } + if token != "" { + co.AuthInfo.Token = token + } + + // 3) namespace within that cluster + if namespace != "" { + co.Context.Namespace = namespace + } return clientcmd.NewNonInteractiveDeferredLoadingClientConfig( clientcmd.NewDefaultClientConfigLoadingRules(), - &clientcmd.ConfigOverrides{}) + &co), nil +} + +// authEntryOverrides builds ConfigOverrides from stored auth in local.yaml. +func authEntryOverrides(entry *fn.AuthEntry) clientcmd.ConfigOverrides { + co := clientcmd.ConfigOverrides{} + co.ClusterInfo.CertificateAuthorityData = decodeBase64(entry.Cluster.CertificateAuthorityData) + co.ClusterInfo.CertificateAuthority = entry.Cluster.CertificateAuthority + co.ClusterInfo.InsecureSkipTLSVerify = entry.Cluster.InsecureSkipTLSVerify + + co.AuthInfo.ClientCertificateData = decodeBase64(entry.User.ClientCertificateData) + co.AuthInfo.ClientCertificate = entry.User.ClientCertificate + co.AuthInfo.ClientKeyData = decodeBase64(entry.User.ClientKeyData) + co.AuthInfo.ClientKey = entry.User.ClientKey + co.AuthInfo.Token = entry.User.Token + return co +} + +// resolveKubeconfigAuth searches the kubeconfig for a context whose cluster +// URL matches the target and returns overrides with that context's auth. +// +// Priority: +// 1. Active context's cluster URL matches → use its auth. +// 2. Exactly one other context matches → use its auth. +// 3. Zero or multiple matches → error. +func resolveKubeconfigAuth(targetURL string) (clientcmd.ConfigOverrides, error) { + loader := clientcmd.NewDefaultClientConfigLoadingRules() + raw, err := loader.Load() + if err != nil { + return clientcmd.ConfigOverrides{}, fmt.Errorf("failed to load kubeconfig: %w", err) + } + + target := strings.TrimRight(targetURL, "/") + + // Check active context first: if it already targets the cluster, prefer it + // (even if other contexts also match). + if ctxName := raw.CurrentContext; ctxName != "" { + if ctx, ok := raw.Contexts[ctxName]; ok { + if cluster, ok := raw.Clusters[ctx.Cluster]; ok && strings.TrimRight(cluster.Server, "/") == target { + fmt.Fprintf(os.Stderr, "Using active kubeconfig context %q for cluster %s\n", ctxName, targetURL) + return kubeconfigAuthOverrides(raw, ctx), nil + } + } + } + + // Otherwise require exactly one other context whose cluster URL matches. + var matchNames []string + for name, ctx := range raw.Contexts { + if cluster, ok := raw.Clusters[ctx.Cluster]; ok && strings.TrimRight(cluster.Server, "/") == target { + matchNames = append(matchNames, name) + } + } + + switch len(matchNames) { + case 0: + return clientcmd.ConfigOverrides{}, fmt.Errorf("no kubeconfig context found for cluster URL %q; pass --cluster-token or ensure a kubeconfig context targets this cluster", targetURL) + case 1: + fmt.Fprintf(os.Stderr, "Using kubeconfig context %q for cluster %s\n", matchNames[0], targetURL) + return kubeconfigAuthOverrides(raw, raw.Contexts[matchNames[0]]), nil + default: + return clientcmd.ConfigOverrides{}, fmt.Errorf("multiple kubeconfig contexts (%s) match cluster URL %q; disambiguate with --cluster-token, switch to correct context or a stored credential", strings.Join(matchNames, ", "), targetURL) + } +} + +// kubeconfigAuthOverrides builds ConfigOverrides from a kubeconfig context. +func kubeconfigAuthOverrides(raw *clientcmdapi.Config, ctx *clientcmdapi.Context) clientcmd.ConfigOverrides { + co := clientcmd.ConfigOverrides{} + co.Context.Namespace = ctx.Namespace + if cluster, ok := raw.Clusters[ctx.Cluster]; ok { + co.ClusterInfo.CertificateAuthorityData = cluster.CertificateAuthorityData + co.ClusterInfo.CertificateAuthority = cluster.CertificateAuthority + co.ClusterInfo.InsecureSkipTLSVerify = cluster.InsecureSkipTLSVerify + } + if authInfo, ok := raw.AuthInfos[ctx.AuthInfo]; ok { + co.AuthInfo.ClientCertificateData = authInfo.ClientCertificateData + co.AuthInfo.ClientCertificate = authInfo.ClientCertificate + co.AuthInfo.ClientKeyData = authInfo.ClientKeyData + co.AuthInfo.ClientKey = authInfo.ClientKey + co.AuthInfo.Token = authInfo.Token + co.AuthInfo.Exec = authInfo.Exec + } + return co +} + +func decodeBase64(s string) []byte { + if s == "" { + return nil + } + b, err := base64.StdEncoding.DecodeString(s) + if err != nil { + // just warn here; if user edits local.yaml file manually they get a + // warning here. Adding an error would mean propagating into every call + // site of the client initialization. + fmt.Fprintf(os.Stderr, "Warning: failed to decode base64 credential data from local.yaml: %v\n", err) + return nil + } + return b } diff --git a/pkg/k8s/client_test.go b/pkg/k8s/client_test.go new file mode 100644 index 0000000000..e42493097f --- /dev/null +++ b/pkg/k8s/client_test.go @@ -0,0 +1,844 @@ +package k8s + +import ( + "encoding/base64" + "io" + "os" + "path/filepath" + "testing" + + "gopkg.in/yaml.v2" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + + fn "knative.dev/func/pkg/functions" +) + +// writeTestKubeconfig serializes a clientcmdapi.Config to a temp file and +// points the KUBECONFIG env var at it for the duration of the test. +func writeTestKubeconfig(t *testing.T, cfg clientcmdapi.Config) string { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "kubeconfig") + if err := clientcmd.WriteToFile(cfg, path); err != nil { + t.Fatalf("failed to write test kubeconfig: %v", err) + } + t.Setenv("KUBECONFIG", path) + return path +} + +func testKubeconfig() clientcmdapi.Config { + return clientcmdapi.Config{ + CurrentContext: "test-ctx", + Contexts: map[string]*clientcmdapi.Context{ + "test-ctx": {Cluster: "test-cluster", AuthInfo: "test-user", Namespace: "test-ns"}, + }, + Clusters: map[string]*clientcmdapi.Cluster{ + "test-cluster": { + Server: "https://kubeconfig.example.com:6443", + CertificateAuthorityData: []byte("-----BEGIN CERTIFICATE-----\nCA-DATA\n-----END CERTIFICATE-----"), + }, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "test-user": { + Token: "kubeconfig-token", + }, + }, + } +} + +func testKubeconfigWithCerts() clientcmdapi.Config { + return clientcmdapi.Config{ + CurrentContext: "cert-ctx", + Contexts: map[string]*clientcmdapi.Context{ + "cert-ctx": {Cluster: "cert-cluster", AuthInfo: "cert-user", Namespace: "cert-ns"}, + }, + Clusters: map[string]*clientcmdapi.Cluster{ + "cert-cluster": { + Server: "https://cert.example.com:6443", + CertificateAuthorityData: []byte("-----BEGIN CERTIFICATE-----\nCA-PEM\n-----END CERTIFICATE-----"), + }, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "cert-user": { + ClientCertificateData: []byte("-----BEGIN CERTIFICATE-----\nCLIENT-CERT-PEM\n-----END CERTIFICATE-----"), + ClientKeyData: []byte("-----BEGIN RSA PRIVATE KEY-----\nCLIENT-KEY-PEM\n-----END RSA PRIVATE KEY-----"), + }, + }, + } +} + +// --- BuildClientConfig tests --- + +func TestBuildClientConfig_NoOverride_UsesKubeconfig(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + cc, _ := BuildClientConfig("", "", "", fn.Local{}) + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.Host != "https://kubeconfig.example.com:6443" { + t.Fatalf("expected kubeconfig host, got %s", cfg.Host) + } + if cfg.BearerToken != "kubeconfig-token" { + t.Fatalf("expected kubeconfig token, got %s", cfg.BearerToken) + } +} + +func TestBuildClientConfig_NoOverride_Namespace(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + cc, _ := BuildClientConfig("", "", "", fn.Local{}) + ns, _, err := cc.Namespace() + if err != nil { + t.Fatal(err) + } + if ns != "test-ns" { + t.Fatalf("expected test-ns, got %s", ns) + } +} + +// TestBuildClientConfig_NamespacePin verifies that an explicit namespace (the +// function's f.Deploy.Namespace) pins the client's default namespace, winning +// over the active kubeconfig context. This is the namespace leg of the context +// pin: once set, cluster operations no longer follow `kubens`/active-context +// changes. (The empty case staying on the active "test-ns" is covered above.) +func TestBuildClientConfig_NamespacePin(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) // active context namespace = "test-ns" + + cc, err := BuildClientConfig("", "", "pinned-ns", fn.Local{}) + if err != nil { + t.Fatal(err) + } + ns, _, err := cc.Namespace() + if err != nil { + t.Fatal(err) + } + if ns != "pinned-ns" { + t.Fatalf("namespace = %q, want pinned-ns (must beat active context test-ns)", ns) + } +} + +// nsContextKubeconfig has two namespaced contexts: the active one targets +// cluster-a (namespace "active-ns"), a non-active one targets cluster-b +// (namespace "other-ns"). +func nsContextKubeconfig() clientcmdapi.Config { + return clientcmdapi.Config{ + CurrentContext: "active-ctx", + Contexts: map[string]*clientcmdapi.Context{ + "active-ctx": {Cluster: "cluster-a", AuthInfo: "user-a", Namespace: "active-ns"}, + "other-ctx": {Cluster: "cluster-b", AuthInfo: "user-b", Namespace: "other-ns"}, + }, + Clusters: map[string]*clientcmdapi.Cluster{ + "cluster-a": {Server: "https://cluster-a.example.com:6443"}, + "cluster-b": {Server: "https://cluster-b.example.com:6443"}, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "user-a": {Token: "token-a"}, + "user-b": {Token: "token-b"}, + }, + } +} + +// TestBuildClientConfig_NamespacePin_AdoptsMatchedContext verifies that when a +// cluster URL resolves to a (non-active) kubeconfig context, that context's +// namespace is adopted -- not the active context's. This is the kubeconfigAuth- +// Overrides namespace leg. +func TestBuildClientConfig_NamespacePin_AdoptsMatchedContext(t *testing.T) { + writeTestKubeconfig(t, nsContextKubeconfig()) + + // Resolve by cluster-b's URL (the non-active context, namespace "other-ns"). + cc, err := BuildClientConfig("https://cluster-b.example.com:6443", "", "", fn.Local{}) + if err != nil { + t.Fatal(err) + } + ns, _, err := cc.Namespace() + if err != nil { + t.Fatal(err) + } + if ns != "other-ns" { + t.Fatalf("namespace = %q, want other-ns (the matched context's namespace, not active active-ns)", ns) + } +} + +// TestBuildClientConfig_NamespacePin_ExplicitBeatsMatchedContext verifies the +// top of the precedence ladder: an explicit namespace wins even over the +// matched context's own namespace. +func TestBuildClientConfig_NamespacePin_ExplicitBeatsMatchedContext(t *testing.T) { + writeTestKubeconfig(t, nsContextKubeconfig()) + + cc, err := BuildClientConfig("https://cluster-b.example.com:6443", "", "explicit-ns", fn.Local{}) + if err != nil { + t.Fatal(err) + } + ns, _, err := cc.Namespace() + if err != nil { + t.Fatal(err) + } + if ns != "explicit-ns" { + t.Fatalf("namespace = %q, want explicit-ns (must beat matched context's other-ns)", ns) + } +} + +func TestBuildClientConfig_ClusterOverride_WithStoredAuth(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + local := fn.Local{ + Auth: []fn.AuthEntry{{ + ClusterURL: "https://override.example.com:6443", + User: fn.UserAuth{Token: "override-tok"}, + }}, + } + cc, _ := BuildClientConfig("https://override.example.com:6443", "", "", local) + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.Host != "https://override.example.com:6443" { + t.Fatalf("expected override host, got %s", cfg.Host) + } +} + +func TestBuildClientConfig_ClusterOverride_NoAuth_Errors(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + _, err := BuildClientConfig("https://unknown.example.com:6443", "", "", fn.Local{}) + if err == nil { + t.Fatal("expected error for cluster URL with no auth source") + } +} + +func TestBuildClientConfig_ClusterOverride_MatchesKubeconfig(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + cc, err := BuildClientConfig("https://kubeconfig.example.com:6443", "", "", fn.Local{}) + if err != nil { + t.Fatal(err) + } + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.Host != "https://kubeconfig.example.com:6443" { + t.Fatalf("expected kubeconfig host, got %s", cfg.Host) + } + if cfg.BearerToken != "kubeconfig-token" { + t.Fatalf("expected kubeconfig token resolved via URL match, got %s", cfg.BearerToken) + } +} + +func TestBuildClientConfig_TokenOverride(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + cc, _ := BuildClientConfig("https://override.example.com:6443", "my-token", "", fn.Local{}) + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.BearerToken != "my-token" { + t.Fatalf("expected my-token, got %s", cfg.BearerToken) + } +} + +func TestBuildClientConfig_TokenWithoutCluster_AppliedToKubeconfig(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + cc, _ := BuildClientConfig("", "override-token", "", fn.Local{}) + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.BearerToken != "override-token" { + t.Fatalf("expected override-token to apply even without cluster URL, got %s", cfg.BearerToken) + } + if cfg.Host != "https://kubeconfig.example.com:6443" { + t.Fatalf("expected kubeconfig host unchanged, got %s", cfg.Host) + } +} + +func TestBuildClientConfig_StoredAuth_Token(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + local := fn.Local{ + Auth: []fn.AuthEntry{{ + ClusterURL: "https://stored.example.com:6443", + User: fn.UserAuth{Token: "stored-token"}, + }}, + } + + cc, _ := BuildClientConfig("https://stored.example.com:6443", "", "", local) + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.Host != "https://stored.example.com:6443" { + t.Fatalf("expected stored host, got %s", cfg.Host) + } + if cfg.BearerToken != "stored-token" { + t.Fatalf("expected stored-token, got %s", cfg.BearerToken) + } +} + +func TestBuildClientConfig_StoredAuth_CertData(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + caRaw := []byte("-----BEGIN CERTIFICATE-----\nMY-CA\n-----END CERTIFICATE-----") + certRaw := []byte("-----BEGIN CERTIFICATE-----\nMY-CERT\n-----END CERTIFICATE-----") + keyRaw := []byte("-----BEGIN RSA PRIVATE KEY-----\nMY-KEY\n-----END RSA PRIVATE KEY-----") + + local := fn.Local{ + Auth: []fn.AuthEntry{{ + ClusterURL: "https://certs.example.com", + Cluster: fn.ClusterTLS{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString(caRaw), + }, + User: fn.UserAuth{ + ClientCertificateData: base64.StdEncoding.EncodeToString(certRaw), + ClientKeyData: base64.StdEncoding.EncodeToString(keyRaw), + }, + }}, + } + + cc, _ := BuildClientConfig("https://certs.example.com", "", "", local) + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if string(cfg.CAData) != string(caRaw) { + t.Fatalf("CAData mismatch: got %q", cfg.CAData) + } + if string(cfg.CertData) != string(certRaw) { + t.Fatalf("CertData mismatch: got %q", cfg.CertData) + } + if string(cfg.KeyData) != string(keyRaw) { + t.Fatalf("KeyData mismatch: got %q", cfg.KeyData) + } +} + +func TestBuildClientConfig_StoredAuth_InsecureSkipTLS(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + local := fn.Local{ + Auth: []fn.AuthEntry{{ + ClusterURL: "https://insecure.example.com", + Cluster: fn.ClusterTLS{ + InsecureSkipTLSVerify: true, + }, + User: fn.UserAuth{Token: "tok"}, + }}, + } + + cc, _ := BuildClientConfig("https://insecure.example.com", "", "", local) + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if !cfg.Insecure { + t.Fatal("expected Insecure=true") + } +} + +func TestBuildClientConfig_StoredAuth_CertFilePaths(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + dir := t.TempDir() + caFile := filepath.Join(dir, "ca.crt") + certFile := filepath.Join(dir, "client.crt") + keyFile := filepath.Join(dir, "client.key") + + if err := os.WriteFile(caFile, []byte("CA-FILE-DATA"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(certFile, []byte("CERT-FILE-DATA"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(keyFile, []byte("KEY-FILE-DATA"), 0600); err != nil { + t.Fatal(err) + } + + local := fn.Local{ + Auth: []fn.AuthEntry{{ + ClusterURL: "https://filepaths.example.com", + Cluster: fn.ClusterTLS{ + CertificateAuthority: caFile, + }, + User: fn.UserAuth{ + ClientCertificate: certFile, + ClientKey: keyFile, + Token: "file-tok", + }, + }}, + } + + cc, _ := BuildClientConfig("https://filepaths.example.com", "", "", local) + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.CAFile != caFile { + t.Fatalf("expected CAFile %s, got %s", caFile, cfg.CAFile) + } + if cfg.CertFile != certFile { + t.Fatalf("expected CertFile %s, got %s", certFile, cfg.CertFile) + } + if cfg.KeyFile != keyFile { + t.Fatalf("expected KeyFile %s, got %s", keyFile, cfg.KeyFile) + } +} + +func TestBuildClientConfig_TokenOverride_BeatsStoredToken(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + local := fn.Local{ + Auth: []fn.AuthEntry{{ + ClusterURL: "https://stored.example.com", + User: fn.UserAuth{Token: "stored-token"}, + }}, + } + + cc, _ := BuildClientConfig("https://stored.example.com", "flag-token", "", local) + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.BearerToken != "flag-token" { + t.Fatalf("expected flag-token to win, got %s", cfg.BearerToken) + } +} + +func TestBuildClientConfig_ClusterOverride_WithToken_SkipsKubeconfigSearch(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + + // When a token is provided, kubeconfig search is skipped even if URL doesn't match + cc, err := BuildClientConfig("https://unknown.example.com", "my-token", "", fn.Local{}) + if err != nil { + t.Fatal(err) + } + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.Host != "https://unknown.example.com" { + t.Fatalf("expected override host, got %s", cfg.Host) + } + if cfg.BearerToken != "my-token" { + t.Fatalf("expected my-token, got %s", cfg.BearerToken) + } +} + +// --- resolveKubeconfigAuth tests --- + +func multiContextKubeconfig() clientcmdapi.Config { + return clientcmdapi.Config{ + CurrentContext: "active-ctx", + Contexts: map[string]*clientcmdapi.Context{ + "active-ctx": {Cluster: "cluster-a", AuthInfo: "user-a"}, + "other-ctx": {Cluster: "cluster-b", AuthInfo: "user-b"}, + }, + Clusters: map[string]*clientcmdapi.Cluster{ + "cluster-a": {Server: "https://cluster-a.example.com:6443"}, + "cluster-b": {Server: "https://cluster-b.example.com:6443"}, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "user-a": {Token: "token-a"}, + "user-b": {Token: "token-b"}, + }, + } +} + +func TestBuildClientConfig_URLResolution_ActiveContext(t *testing.T) { + writeTestKubeconfig(t, multiContextKubeconfig()) + + cc, err := BuildClientConfig("https://cluster-a.example.com:6443", "", "", fn.Local{}) + if err != nil { + t.Fatal(err) + } + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.BearerToken != "token-a" { + t.Fatalf("expected active context token, got %s", cfg.BearerToken) + } +} + +func TestBuildClientConfig_URLResolution_NonActiveContext(t *testing.T) { + writeTestKubeconfig(t, multiContextKubeconfig()) + + cc, err := BuildClientConfig("https://cluster-b.example.com:6443", "", "", fn.Local{}) + if err != nil { + t.Fatal(err) + } + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.BearerToken != "token-b" { + t.Fatalf("expected non-active context token, got %s", cfg.BearerToken) + } +} + +func TestBuildClientConfig_URLResolution_MultipleMatches(t *testing.T) { + writeTestKubeconfig(t, clientcmdapi.Config{ + CurrentContext: "ctx-1", + Contexts: map[string]*clientcmdapi.Context{ + "ctx-1": {Cluster: "cluster-x", AuthInfo: "user-1"}, + "ctx-2": {Cluster: "cluster-y", AuthInfo: "user-2"}, + "ctx-3": {Cluster: "cluster-z", AuthInfo: "user-3"}, + }, + Clusters: map[string]*clientcmdapi.Cluster{ + "cluster-x": {Server: "https://active.example.com:6443"}, + "cluster-y": {Server: "https://shared.example.com:6443"}, + "cluster-z": {Server: "https://shared.example.com:6443"}, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "user-1": {Token: "tok-1"}, + "user-2": {Token: "tok-2"}, + "user-3": {Token: "tok-3"}, + }, + }) + + _, err := BuildClientConfig("https://shared.example.com:6443", "", "", fn.Local{}) + if err == nil { + t.Fatal("expected error for multiple matching contexts") + } +} + +func TestBuildClientConfig_URLResolution_NoMatch(t *testing.T) { + writeTestKubeconfig(t, multiContextKubeconfig()) + + _, err := BuildClientConfig("https://nonexistent.example.com:6443", "", "", fn.Local{}) + if err == nil { + t.Fatal("expected error for no matching context") + } +} + +func TestBuildClientConfig_URLResolution_TrailingSlashNormalized(t *testing.T) { + writeTestKubeconfig(t, multiContextKubeconfig()) + + // target has a trailing slash; the kubeconfig server does not -> must match. + cc, err := BuildClientConfig("https://cluster-a.example.com:6443/", "", "", fn.Local{}) + if err != nil { + t.Fatalf("trailing-slash target failed to resolve: %v", err) + } + cfg, err := cc.ClientConfig() + if err != nil { + t.Fatal(err) + } + if cfg.BearerToken != "token-a" { + t.Fatalf("expected token-a via trailing-slash match, got %q", cfg.BearerToken) + } +} + +func TestBuildClientConfig_URLResolution_MultipleMatches_ErrorListsContexts(t *testing.T) { + writeTestKubeconfig(t, clientcmdapi.Config{ + CurrentContext: "ctx-1", + Contexts: map[string]*clientcmdapi.Context{ + "ctx-1": {Cluster: "cluster-x", AuthInfo: "user-1"}, + "ctx-2": {Cluster: "cluster-y", AuthInfo: "user-2"}, + "ctx-3": {Cluster: "cluster-z", AuthInfo: "user-3"}, + }, + Clusters: map[string]*clientcmdapi.Cluster{ + "cluster-x": {Server: "https://active.example.com:6443"}, + "cluster-y": {Server: "https://shared.example.com:6443"}, + "cluster-z": {Server: "https://shared.example.com:6443"}, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "user-1": {Token: "tok-1"}, + "user-2": {Token: "tok-2"}, + "user-3": {Token: "tok-3"}, + }, + }) + + _, err := BuildClientConfig("https://shared.example.com:6443", "", "", fn.Local{}) + if err == nil { + t.Fatal("expected error for multiple matching contexts") + } + // The improved error names the ambiguous contexts and how to disambiguate. + for _, want := range []string{"ctx-2", "ctx-3", "disambiguate"} { + if !contains(err.Error(), want) { + t.Fatalf("multi-match error missing %q: %v", want, err) + } + } +} + +func TestResolveKubeconfigAuth_NoteOnActiveMatch(t *testing.T) { + writeTestKubeconfig(t, multiContextKubeconfig()) + out := captureStderr(t, func() { + if _, err := BuildClientConfig("https://cluster-a.example.com:6443", "", "", fn.Local{}); err != nil { + t.Fatal(err) + } + }) + if !contains(out, `Using active kubeconfig context "active-ctx"`) { + t.Fatalf("expected active-context note, got %q", out) + } +} + +func TestResolveKubeconfigAuth_NoteOnNonActiveMatch(t *testing.T) { + writeTestKubeconfig(t, multiContextKubeconfig()) + out := captureStderr(t, func() { + if _, err := BuildClientConfig("https://cluster-b.example.com:6443", "", "", fn.Local{}); err != nil { + t.Fatal(err) + } + }) + if !contains(out, `Using kubeconfig context "other-ctx"`) { + t.Fatalf("expected non-active-context note, got %q", out) + } +} + +// captureStderr redirects os.Stderr for the duration of f and returns what was +// written (resolveKubeconfigAuth prints its selection note to os.Stderr). +func captureStderr(t *testing.T, f func()) string { + t.Helper() + old := os.Stderr + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stderr = w + defer func() { os.Stderr = old }() + f() + _ = w.Close() + b, err := io.ReadAll(r) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +// --- YAML round-trip tests for []byte fields --- + +func TestAuthEntry_YAMLRoundTrip(t *testing.T) { + caB64 := base64.StdEncoding.EncodeToString([]byte("CA-DATA")) + certB64 := base64.StdEncoding.EncodeToString([]byte("CLIENT-CERT")) + keyB64 := base64.StdEncoding.EncodeToString([]byte("CLIENT-KEY")) + + original := fn.Local{ + Auth: []fn.AuthEntry{{ + ClusterURL: "https://roundtrip.example.com", + Cluster: fn.ClusterTLS{ + CertificateAuthorityData: caB64, + InsecureSkipTLSVerify: true, + }, + User: fn.UserAuth{ + ClientCertificateData: certB64, + ClientKeyData: keyB64, + Token: "my-token", + }, + }}, + } + + // Marshal to YAML + data, err := yaml.Marshal(&original) + if err != nil { + t.Fatalf("marshal failed: %v", err) + } + + // Verify the base64 string is present in YAML output + yamlStr := string(data) + if !contains(yamlStr, caB64) { + t.Fatalf("expected base64 CA data in YAML output, got:\n%s", yamlStr) + } + + // Unmarshal back + var restored fn.Local + if err := yaml.Unmarshal(data, &restored); err != nil { + t.Fatalf("unmarshal failed: %v", err) + } + + if len(restored.Auth) != 1 { + t.Fatalf("expected 1 auth entry, got %d", len(restored.Auth)) + } + entry := restored.Auth[0] + + if entry.ClusterURL != "https://roundtrip.example.com" { + t.Fatalf("ClusterURL mismatch: %s", entry.ClusterURL) + } + if entry.Cluster.CertificateAuthorityData != caB64 { + t.Fatalf("CertificateAuthorityData mismatch: got %q", entry.Cluster.CertificateAuthorityData) + } + if !entry.Cluster.InsecureSkipTLSVerify { + t.Fatal("expected InsecureSkipTLSVerify=true") + } + if entry.User.ClientCertificateData != certB64 { + t.Fatalf("ClientCertificateData mismatch: got %q", entry.User.ClientCertificateData) + } + if entry.User.ClientKeyData != keyB64 { + t.Fatalf("ClientKeyData mismatch: got %q", entry.User.ClientKeyData) + } + if entry.User.Token != "my-token" { + t.Fatalf("Token mismatch: got %s", entry.User.Token) + } +} + +func TestAuthEntry_YAMLRoundTrip_WithFilePaths(t *testing.T) { + original := fn.Local{ + Auth: []fn.AuthEntry{{ + ClusterURL: "https://paths.example.com", + Cluster: fn.ClusterTLS{ + CertificateAuthority: "/path/to/ca.crt", + }, + User: fn.UserAuth{ + ClientCertificate: "/path/to/client.crt", + ClientKey: "/path/to/client.key", + Token: "path-token", + }, + }}, + } + + data, err := yaml.Marshal(&original) + if err != nil { + t.Fatalf("marshal failed: %v", err) + } + + var restored fn.Local + if err := yaml.Unmarshal(data, &restored); err != nil { + t.Fatalf("unmarshal failed: %v", err) + } + + entry := restored.Auth[0] + if entry.Cluster.CertificateAuthority != "/path/to/ca.crt" { + t.Fatalf("CertificateAuthority mismatch: %s", entry.Cluster.CertificateAuthority) + } + if entry.User.ClientCertificate != "/path/to/client.crt" { + t.Fatalf("ClientCertificate mismatch: %s", entry.User.ClientCertificate) + } + if entry.User.ClientKey != "/path/to/client.key" { + t.Fatalf("ClientKey mismatch: %s", entry.User.ClientKey) + } +} + +// --- Kubeconfig-to-Local conversion test --- + +func TestKubeconfigToLocal_CertDataRoundTrip(t *testing.T) { + kubecfg := testKubeconfigWithCerts() + writeTestKubeconfig(t, kubecfg) + + cc, _ := BuildClientConfig("", "", "", fn.Local{}) + kc := NewClient(cc) + + // Extract into our types using Auth() + url, clusterTLS, user, err := kc.Auth() + if err != nil { + t.Fatal(err) + } + + // Verify the base64 decodes back to what we put in + decoded, _ := base64.StdEncoding.DecodeString(clusterTLS.CertificateAuthorityData) + if string(decoded) != "-----BEGIN CERTIFICATE-----\nCA-PEM\n-----END CERTIFICATE-----" { + t.Fatalf("CA data mismatch: %q", decoded) + } + + // Feed back into BuildClientConfig and verify it produces matching rest.Config + local := fn.Local{} + local.SetAuth(url, clusterTLS, user) + cc2, _ := BuildClientConfig(url, "", "", local) + restCfg, err := cc2.ClientConfig() + if err != nil { + t.Fatal(err) + } + + if restCfg.Host != url { + t.Fatalf("host mismatch: got %s", restCfg.Host) + } + if string(restCfg.CAData) != "-----BEGIN CERTIFICATE-----\nCA-PEM\n-----END CERTIFICATE-----" { + t.Fatalf("CAData mismatch after round-trip") + } + if string(restCfg.CertData) != "-----BEGIN CERTIFICATE-----\nCLIENT-CERT-PEM\n-----END CERTIFICATE-----" { + t.Fatalf("CertData mismatch after round-trip") + } + if string(restCfg.KeyData) != "-----BEGIN RSA PRIVATE KEY-----\nCLIENT-KEY-PEM\n-----END RSA PRIVATE KEY-----" { + t.Fatalf("KeyData mismatch after round-trip") + } +} + +// --- Auth() tests --- + +func TestAuth_Token(t *testing.T) { + writeTestKubeconfig(t, testKubeconfig()) + cc, _ := BuildClientConfig("", "", "", fn.Local{}) + kc := NewClient(cc) + + url, clusterTLS, user, err := kc.Auth() + if err != nil { + t.Fatal(err) + } + if url != "https://kubeconfig.example.com:6443" { + t.Fatalf("expected cluster URL, got %s", url) + } + if user.Token != "kubeconfig-token" { + t.Fatalf("expected token, got %s", user.Token) + } + decoded := decodeBase64(clusterTLS.CertificateAuthorityData) + if string(decoded) != "-----BEGIN CERTIFICATE-----\nCA-DATA\n-----END CERTIFICATE-----" { + t.Fatalf("unexpected CA data: %s", decoded) + } +} + +func TestAuth_ClientCerts(t *testing.T) { + writeTestKubeconfig(t, testKubeconfigWithCerts()) + cc, _ := BuildClientConfig("", "", "", fn.Local{}) + kc := NewClient(cc) + + url, _, user, err := kc.Auth() + if err != nil { + t.Fatal(err) + } + if url != "https://cert.example.com:6443" { + t.Fatalf("unexpected URL: %s", url) + } + + certDecoded := decodeBase64(user.ClientCertificateData) + if string(certDecoded) != "-----BEGIN CERTIFICATE-----\nCLIENT-CERT-PEM\n-----END CERTIFICATE-----" { + t.Fatalf("unexpected ClientCertificateData: %s", certDecoded) + } + keyDecoded := decodeBase64(user.ClientKeyData) + if string(keyDecoded) != "-----BEGIN RSA PRIVATE KEY-----\nCLIENT-KEY-PEM\n-----END RSA PRIVATE KEY-----" { + t.Fatalf("unexpected ClientKeyData: %s", keyDecoded) + } +} + +func TestAuth_RoundTrip(t *testing.T) { + writeTestKubeconfig(t, testKubeconfigWithCerts()) + cc, _ := BuildClientConfig("", "", "", fn.Local{}) + kc := NewClient(cc) + + url, clusterTLS, user, err := kc.Auth() + if err != nil { + t.Fatal(err) + } + + // Store in Local and rebuild client + local := fn.Local{} + local.SetAuth(url, clusterTLS, user) + cc2, _ := BuildClientConfig(url, "", "", local) + kc2 := NewClient(cc2) + restCfg, err := kc2.ClientConfig() + if err != nil { + t.Fatal(err) + } + + if restCfg.Host != "https://cert.example.com:6443" { + t.Fatalf("host mismatch: %s", restCfg.Host) + } + if string(restCfg.CertData) != "-----BEGIN CERTIFICATE-----\nCLIENT-CERT-PEM\n-----END CERTIFICATE-----" { + t.Fatalf("CertData mismatch after round-trip") + } + if string(restCfg.KeyData) != "-----BEGIN RSA PRIVATE KEY-----\nCLIENT-KEY-PEM\n-----END RSA PRIVATE KEY-----" { + t.Fatalf("KeyData mismatch after round-trip") + } +} + +func contains(s, substr string) bool { + return len(s) > 0 && len(substr) > 0 && stringContains(s, substr) +} + +func stringContains(s, substr string) bool { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} 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..5e333523d2 100644 --- a/pkg/k8s/configmaps_test.go +++ b/pkg/k8s/configmaps_test.go @@ -3,12 +3,15 @@ package k8s_test import ( "testing" + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" ) func TestListConfigMapsNamesIfConnectedWrongKubeconfig(t *testing.T) { t.Setenv("KUBECONFIG", "/tmp/non-existent.config") - _, err := k8s.ListConfigMapsNamesIfConnected(t.Context(), "") + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + _, err := k8s.ListConfigMapsNamesIfConnected(t.Context(), kc, "") if err != nil { t.Fatal(err) } @@ -16,7 +19,9 @@ func TestListConfigMapsNamesIfConnectedWrongKubeconfig(t *testing.T) { func TestListConfigMapsNamesIfConnectedWrongKubernentesMaster(t *testing.T) { t.Setenv("KUBERNETES_MASTER", "/tmp/non-existent.config") - _, err := k8s.ListConfigMapsNamesIfConnected(t.Context(), "") + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + _, err := k8s.ListConfigMapsNamesIfConnected(t.Context(), kc, "") if err != nil { t.Fatal(err) } diff --git a/pkg/k8s/deployer.go b/pkg/k8s/deployer.go index 1341515ac8..f87c83cbc0 100644 --- a/pkg/k8s/deployer.go +++ b/pkg/k8s/deployer.go @@ -47,10 +47,11 @@ type DeployerOpt func(*Deployer) type Deployer struct { verbose bool decorator deployer.DeployDecorator + kc *Client } -func NewDeployer(opts ...DeployerOpt) *Deployer { - d := &Deployer{} +func NewDeployer(kc *Client, opts ...DeployerOpt) *Deployer { + d := &Deployer{kc: kc} for _, opt := range opts { opt(d) } @@ -69,20 +70,6 @@ func WithDeployerDecorator(decorator deployer.DeployDecorator) DeployerOpt { } } -func 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 - // configuration. We should figure out how to transmit this information - // to the pipeline run for initial builds. This is a new problem because - // earlier versions of this logic relied entirely on the current - // kubernetes context. - if f.Namespace == "" && f.Deploy.Namespace == "" { - f.Namespace, _ = GetDefaultNamespace() - } - return f -} - // newEventingClient creates a Knative Eventing client from a REST config func newEventingClient(config *rest.Config, namespace string) (clienteventingv1.KnEventingClient, error) { eventingClient, err := eventingv1client.NewForConfig(config) @@ -93,7 +80,6 @@ func newEventingClient(config *rest.Config, namespace string) (clienteventingv1. } func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResult, error) { - f = 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 @@ -122,7 +108,7 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu } // Get the Kubernetes REST config - config, err := GetClientConfig().ClientConfig() + config, err := d.kc.ClientConfig() if err != nil { return fn.DeploymentResult{}, err } @@ -156,7 +142,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) } @@ -205,7 +191,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) } @@ -490,10 +476,10 @@ func (d *Deployer) generateService(f fn.Function, namespace string, daprInstalle // 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, kc *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, kc, s, namespace) if err != nil { if errors.IsForbidden(err) { errMsg += " Ensure that the service account has the necessary permissions to access the secret.\n" @@ -504,14 +490,14 @@ func CheckResourcesArePresent(ctx context.Context, namespace string, referencedS } for cm := range *referencedConfigMaps { - _, err := GetConfigMap(ctx, cm, namespace) + _, err := GetConfigMap(ctx, kc, 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, kc, pvc, namespace) if err != nil { errMsg += fmt.Sprintf(" referenced PersistentVolumeClaim \"%s\" is not present in namespace \"%s\"\n", pvc, namespace) } @@ -519,14 +505,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, kc, 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, kc, 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 57dac06983..9983ac1427 100644 --- a/pkg/k8s/deployer_int_test.go +++ b/pkg/k8s/deployer_int_test.go @@ -6,72 +6,86 @@ import ( "testing" deployertesting "knative.dev/func/pkg/deployer/testing" + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" ) +func defaultKc() *k8s.Client { + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + return k8s.NewClient(cc) +} + func TestInt_FullPath(t *testing.T) { + kc := defaultKc() deployertesting.TestInt_FullPath(t, - k8s.NewDeployer(k8s.WithDeployerVerbose(false)), - k8s.NewRemover(false), - k8s.NewLister(false), - k8s.NewDescriber(false), + k8s.NewDeployer(kc, k8s.WithDeployerVerbose(false)), + k8s.NewRemover(kc, false), + k8s.NewLister(kc, false), + k8s.NewDescriber(kc, false), k8s.KubernetesDeployerName) } func TestInt_Deploy(t *testing.T) { + kc := defaultKc() 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 := defaultKc() 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) { t.Skip("Kubernetes deploy does not support func subscribe yet") + kc := defaultKc() 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 := defaultKc() 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 := defaultKc() 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 := defaultKc() 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 := defaultKc() 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/describer.go b/pkg/k8s/describer.go index 14f8468fdf..53a594966a 100644 --- a/pkg/k8s/describer.go +++ b/pkg/k8s/describer.go @@ -16,6 +16,7 @@ import ( type Describer struct { verbose bool transport http.RoundTripper + kc *Client } type DescriberOpt func(*Describer) @@ -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) } @@ -40,7 +41,7 @@ func (d *Describer) Describe(ctx context.Context, name, namespace string) (fn.In return fn.Instance{}, fmt.Errorf("function namespace is required when describing %q", name) } - 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..901ba89a66 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 := defaultKc() 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..433eee9e4a 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,9 +47,9 @@ 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, kc *Client) (*contextDialer, error) { c := &contextDialer{ - clientConfig: clientConfig, + configClient: kc, detachChan: make(chan struct{}), } err := c.startDialerPod(ctx) @@ -62,7 +61,7 @@ func NewInClusterDialer(ctx context.Context, clientConfig clientcmd.ClientConfig type contextDialer struct { coreV1 v1.CoreV1Interface - clientConfig clientcmd.ClientConfig + configClient *Client restConf *restclient.Config podName string namespace string @@ -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.configClient.ClientConfig() 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.configClient.DefaultNamespace() if err != nil { return } @@ -291,7 +290,7 @@ func (c *contextDialer) startDialerPod(ctx context.Context) (err error) { Annotations: nil, }, Spec: coreV1.PodSpec{ - SecurityContext: defaultPodSecurityContext(), + SecurityContext: defaultPodSecurityContext(c.configClient), Containers: []coreV1.Container{ { Name: c.podName, @@ -308,7 +307,7 @@ func (c *contextDialer) startDialerPod(ctx context.Context) (err error) { } creatOpts := metaV1.CreateOptions{} - ready := podReady(ctx, c.coreV1, c.podName, c.namespace) + ready := podReady(ctx, c.configClient, c.coreV1, c.podName, c.namespace) _, err = pods.Create(ctx, pod, creatOpts) if err != nil { @@ -401,7 +400,7 @@ func attach(ctx context.Context, restClient restclient.Interface, restConf *rest }) } -func podReady(ctx context.Context, core v1.CoreV1Interface, podName, namespace string) (errChan <-chan error) { +func podReady(ctx context.Context, kc *Client, core v1.CoreV1Interface, podName, namespace string) (errChan <-chan error) { d := make(chan error) errChan = d @@ -433,7 +432,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, _ := GetPodLogs(ctx, kc, namespace, podName, podName) d <- fmt.Errorf("pod prematurely exited (output: %q, exitcode: %d)", msg, status.State.Terminated.ExitCode) return } @@ -555,14 +554,14 @@ func newConn() (*io.PipeReader, *io.PipeWriter, *conn) { return pr1, pw0, rwc } -func NewLazyInitInClusterDialer(clientConfig clientcmd.ClientConfig) *lazyInitInClusterDialer { +func NewLazyInitInClusterDialer(kc *Client) *lazyInitInClusterDialer { return &lazyInitInClusterDialer{ - clientConfig: clientConfig, + kc: kc, } } type lazyInitInClusterDialer struct { - clientConfig clientcmd.ClientConfig + kc *Client contextDialer *contextDialer initErr error o sync.Once @@ -570,7 +569,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.kc) }) 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..ec198df566 100644 --- a/pkg/k8s/dialer_int_test.go +++ b/pkg/k8s/dialer_int_test.go @@ -20,7 +20,8 @@ import ( metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/rand" - "k8s.io/client-go/kubernetes" + + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" ) @@ -35,14 +36,11 @@ func TestInt_DialInClusterService(t *testing.T) { var ctx = t.Context() // Initialize client configuration from kubeconfig or in-cluster config - clientConfig := k8s.GetClientConfig() + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) - // Extract the REST config and create a clientset for API operations - rc, err := clientConfig.ClientConfig() - if err != nil { - t.Fatal(err) - } - cliSet, err := kubernetes.NewForConfig(rc) + // Create a clientset for API operations + cliSet, err := kc.Clientset() if err != nil { t.Fatal(err) } @@ -56,7 +54,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 +155,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 +220,9 @@ func TestInt_DialInClusterService(t *testing.T) { func TestInt_DialUnreachable(t *testing.T) { var ctx = t.Context() - dialer, err := k8s.NewInClusterDialer(ctx, k8s.GetClientConfig()) + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + dialer, err := k8s.NewInClusterDialer(ctx, kc) if err != nil { t.Fatal(err) } @@ -261,12 +261,9 @@ func TestInt_DialUnreachable(t *testing.T) { func TestInt_DialContextExpiry(t *testing.T) { var setupCtx = t.Context() - clientConfig := k8s.GetClientConfig() - rc, err := clientConfig.ClientConfig() - if err != nil { - t.Fatal(err) - } - cliSet, err := kubernetes.NewForConfig(rc) + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + cliSet, err := kc.Clientset() if err != nil { t.Fatal(err) } @@ -275,7 +272,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 +323,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.go b/pkg/k8s/lister.go index 82e0566f88..7ceaffa589 100644 --- a/pkg/k8s/lister.go +++ b/pkg/k8s/lister.go @@ -13,16 +13,18 @@ import ( type Lister struct { verbose bool + kc *Client } -func NewLister(verbose bool) fn.Lister { +func NewLister(kc *Client, verbose bool) fn.Lister { return &Lister{ + kc: kc, verbose: verbose, } } func (l *Lister) List(ctx context.Context, namespace string) ([]fn.ListItem, error) { - clientset, err := NewKubernetesClientset() + clientset, err := l.kc.Clientset() if err != nil { return nil, fmt.Errorf("unable to create k8s client: %v", err) } diff --git a/pkg/k8s/lister_int_test.go b/pkg/k8s/lister_int_test.go index 10feed5a48..04171875f5 100644 --- a/pkg/k8s/lister_int_test.go +++ b/pkg/k8s/lister_int_test.go @@ -10,10 +10,11 @@ import ( ) func TestInt_List(t *testing.T) { + kc := defaultKc() listertesting.TestInt_List(t, - k8s.NewLister(true), - k8s.NewDeployer(k8s.WithDeployerVerbose(true)), - k8s.NewDescriber(true), - k8s.NewRemover(true), + k8s.NewLister(kc, 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 ed06caa1b1..03806eff80 100644 --- a/pkg/k8s/logs.go +++ b/pkg/k8s/logs.go @@ -16,13 +16,13 @@ import ( // 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, kc *Client, namespace, podName, containerName string) (string, error) { podLogOpts := corev1.PodLogOptions{} if containerName != "" { podLogOpts.Container = containerName } - client, namespace, _ := NewClientAndResolvedNamespace(namespace) + client, namespace, _ := kc.ClientAndNamespace(namespace) request := client.CoreV1().Pods(namespace).GetLogs(podName, &podLogOpts) containerLogStream, err := request.Stream(ctx) @@ -46,8 +46,8 @@ func GetPodLogs(ctx context.Context, namespace, podName, containerName string) ( // In addition, filtering on image can be done so only logs for given image are logged. // // This function runs as long as the passed context is active (i.e. it is required cancel the context to stop log gathering). -func GetPodLogsBySelector(ctx context.Context, namespace, labelSelector, containerName, image string, since *time.Time, out io.Writer) error { - client, namespace, err := NewClientAndResolvedNamespace(namespace) +func GetPodLogsBySelector(ctx context.Context, kc *Client, namespace, labelSelector, containerName, image string, since *time.Time, out io.Writer) error { + client, namespace, err := kc.ClientAndNamespace(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 d8b1b77181..7185384306 100644 --- a/pkg/k8s/logs_int_test.go +++ b/pkg/k8s/logs_int_test.go @@ -11,6 +11,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/rand" + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" ) @@ -18,7 +19,9 @@ 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() + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + cliSet, err := kc.Clientset() if err != nil { t.Fatal(err) } @@ -80,7 +83,7 @@ out: time.Sleep(time.Millisecond * 500) } - out, err := k8s.GetPodLogs(ctx, testingNS, testingPodName, testingPodName) + out, err := k8s.GetPodLogs(ctx, kc, testingNS, testingPodName, testingPodName) if err != nil { t.Fatal(err) } diff --git a/pkg/k8s/manifestival.go b/pkg/k8s/manifestival.go index 7909d4f84a..334f029c84 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(kc *Client) (manifestival.Client, error) { + config, err := kc.ClientConfig() if err != nil { return nil, err } diff --git a/pkg/k8s/openshift.go b/pkg/k8s/openshift.go index b537ca47fc..bc50e0f787 100644 --- a/pkg/k8s/openshift.go +++ b/pkg/k8s/openshift.go @@ -6,7 +6,6 @@ import ( "encoding/pem" "errors" "strings" - "sync" "time" v1 "k8s.io/api/core/v1" @@ -24,8 +23,23 @@ const ( openShiftRegistryHostPort = openShiftRegistryHost + ":5000" ) -func GetOpenShiftServiceCA(ctx context.Context) (*x509.Certificate, error) { - client, ns, err := NewClientAndResolvedNamespace("") +// IsOpenshift detects whether the target cluster is OpenShift by checking +// for the route.openshift.io API group. The result is cached per Client instance. +func (c *Client) IsOpenshift() bool { + c.openShiftOnce.Do(func() { + clientset, err := c.Clientset() + if err != nil { + return + } + _, err = clientset.Discovery().ServerResourcesForGroupVersion("route.openshift.io/v1") + // if this group version is found == Openshift cluster + c.openshift = err == nil + }) + return c.openshift +} + +func (c *Client) GetOpenShiftServiceCA(ctx context.Context) (*x509.Certificate, error) { + client, ns, err := c.ClientAndNamespace("") if err != nil { return nil, err } @@ -85,12 +99,11 @@ func GetOpenShiftServiceCA(ctx context.Context) (*x509.Certificate, error) { } } -func GetDefaultOpenShiftRegistry() string { - ns, _ := GetDefaultNamespace() +func GetDefaultOpenShiftRegistry(c *Client) string { + ns, _ := c.DefaultNamespace() if ns == "" { ns = "default" } - return openShiftRegistryHostPort + "/" + ns } @@ -100,23 +113,15 @@ func IsOpenShiftInternalRegistry(registry string) bool { return strings.HasPrefix(registry, openShiftRegistryHost) } -func GetOpenShiftDockerCredentialLoaders() []creds.CredentialsCallback { - conf := GetClientConfig() - - rawConf, err := conf.RawConfig() - if err != nil { - return nil - } - - cc, ok := rawConf.Contexts[rawConf.CurrentContext] - if !ok { +func GetOpenShiftDockerCredentialLoaders(c *Client) []creds.CredentialsCallback { + restCfg, err := c.ClientConfig() + if err != nil || restCfg.BearerToken == "" { return nil } - var credentials oci.Credentials - if authInfo := rawConf.AuthInfos[cc.AuthInfo]; authInfo != nil { - credentials.Username = "openshift" - credentials.Password = authInfo.Token + credentials := oci.Credentials{ + Username: "openshift", + Password: restCfg.BearerToken, } return []creds.CredentialsCallback{ @@ -130,42 +135,6 @@ func GetOpenShiftDockerCredentialLoaders() []creds.CredentialsCallback { } -var isOpenShift bool -var checkOpenShiftOnce sync.Once - -// SetOpenShiftForTest overrides OpenShift detection for testing. -// Returns a cleanup function that restores the previous state. -func SetOpenShiftForTest(val bool) func() { - checkOpenShiftOnce.Do(func() {}) // ensure real detection won't run - prev := isOpenShift - isOpenShift = val - return func() { isOpenShift = prev } -} - -func IsOpenShift() bool { - checkOpenShiftOnce.Do(func() { - isOpenShift = false - client, err := NewKubernetesClientset() - if err != nil { - return - } - - // Detect OpenShift by checking for OpenShift-specific API groups - // This is reliable and works even with restrictive RBAC, unlike checking - // for namespaces/services which can produce false positives when forbidden - discoveryClient := client.Discovery() - - // Check for route.openshift.io API group (Routes are OpenShift-specific) - _, err = discoveryClient.ServerResourcesForGroupVersion("route.openshift.io/v1") - if err == nil { - // API group exists - this is OpenShift - isOpenShift = true - } - // If NotFound or any other error, this is most likely not OpenShift - }) - return isOpenShift -} - 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 c61d7c5aed..cd63fc34c4 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, kc *Client, name, namespaceOverride string) (*corev1.PersistentVolumeClaim, error) { + client, namespace, err := kc.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, kc *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 := kc.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, kc *Client, namespaceOverride string, listOptions metav1.ListOptions) (err error) { + client, namespace, err := kc.ClientAndNamespace(namespaceOverride) if err != nil { return } @@ -72,20 +72,57 @@ func DeletePersistentVolumeClaims(ctx context.Context, namespaceOverride string, return client.CoreV1().PersistentVolumeClaims(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, listOptions) } +// DeletePersistentVolumeClaim deletes a single PVC by name +func DeletePersistentVolumeClaim(ctx context.Context, kc *Client, name, namespaceOverride string) error { + client, namespace, err := kc.ClientAndNamespace(namespaceOverride) + if err != nil { + return err + } + + err = client.CoreV1().PersistentVolumeClaims(namespace).Delete(ctx, name, metav1.DeleteOptions{}) + if err != nil && !k8serrors.IsNotFound(err) { + return fmt.Errorf("failed to delete PVC %s: %w", name, err) + } + return nil +} + +// WaitForPVCDeletion waits for a PVC to be fully deleted (not just in Terminating state) +func WaitForPVCDeletion(ctx context.Context, kc *Client, name, namespaceOverride string) error { + client, namespace, err := kc.ClientAndNamespace(namespaceOverride) + if err != nil { + return err + } + + for { + select { + case <-ctx.Done(): + return ctx.Err() + default: + _, err := client.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, name, metav1.GetOptions{}) + if k8serrors.IsNotFound(err) { + return nil + } + if err != nil { + return fmt.Errorf("error checking PVC deletion status: %w", err) + } + time.Sleep(time.Second) + } + } +} + 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, kc *Client, content io.Reader, claimName, namespace string) error { + return runWithVolumeMounted(ctx, kc, 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, kc *Client, podImage string, podCommand []string, podInput io.Reader, claimName, namespace string) error { var err error - cliConf := GetClientConfig() - restConf, err := cliConf.ClientConfig() + restConf, err := kc.ClientConfig() if err != nil { return fmt.Errorf("cannot get client config: %w", err) } @@ -102,7 +139,7 @@ func runWithVolumeMounted(ctx context.Context, podImage string, podCommand []str } if namespace == "" { - namespace, err = GetDefaultNamespace() + namespace, err = kc.DefaultNamespace() if err != nil { return fmt.Errorf("cannot get namespace: %w", err) } @@ -125,7 +162,7 @@ func runWithVolumeMounted(ctx context.Context, podImage string, podCommand []str Annotations: nil, }, Spec: corev1.PodSpec{ - SecurityContext: defaultPodSecurityContext(), + SecurityContext: defaultPodSecurityContext(kc), Containers: []corev1.Container{ { Name: podName, @@ -159,7 +196,7 @@ func runWithVolumeMounted(ctx context.Context, podImage string, podCommand []str localCtx, cancel := context.WithCancel(ctx) defer cancel() - ready := podReady(localCtx, client.CoreV1(), podName, namespace) + ready := podReady(localCtx, kc, client.CoreV1(), podName, namespace) _, err = pods.Create(ctx, pod, metav1.CreateOptions{}) if err != nil { @@ -240,8 +277,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, kc *Client, namespaceOverride string) (names []string, err error) { + names, err = listPersistentVolumeClaimsNames(ctx, kc, namespaceOverride) if err != nil { // not logged our authorized to access resources if k8serrors.IsForbidden(err) || k8serrors.IsUnauthorized(err) || k8serrors.IsInvalid(err) || k8serrors.IsTimeout(err) { @@ -270,8 +307,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, kc *Client, namespaceOverride string) (names []string, err error) { + client, namespace, err := kc.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..f13f041a00 100644 --- a/pkg/k8s/persistent_volumes_int_test.go +++ b/pkg/k8s/persistent_volumes_int_test.go @@ -16,6 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/util/rand" + "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" ) @@ -24,7 +25,9 @@ func TestInt_UploadToVolume(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), time.Minute*5) t.Cleanup(cancel) - cliSet, testingNS, err := k8s.NewClientAndResolvedNamespace("") + cc, _ := k8s.BuildClientConfig("", "", "", functions.Local{}) + kc := k8s.NewClient(cc) + cliSet, testingNS, err := kc.ClientAndNamespace("") if err != nil { t.Fatal(err) } @@ -32,7 +35,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 +51,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 +62,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 +129,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, kc, testingNS, testingPodName, testingPodName) if err != nil { t.Fatal(err) } @@ -138,7 +141,9 @@ func TestInt_UploadToVolume(t *testing.T) { func TestInt_ListPersistentVolumeClaimsNamesIfConnectedWrongKubeconfig(t *testing.T) { t.Setenv("KUBECONFIG", "/tmp/non-existent.config") - _, err := k8s.ListPersistentVolumeClaimsNamesIfConnected(t.Context(), "") + cc, _ := k8s.BuildClientConfig("", "", "", functions.Local{}) + kc := k8s.NewClient(cc) + _, err := k8s.ListPersistentVolumeClaimsNamesIfConnected(t.Context(), kc, "") if err != nil { t.Fatal(err) } @@ -146,7 +151,9 @@ func TestInt_ListPersistentVolumeClaimsNamesIfConnectedWrongKubeconfig(t *testin func TestInt_ListPersistentVolumeClaimsNamesIfConnectedWrongKubernentesMaster(t *testing.T) { t.Setenv("KUBERNETES_MASTER", "/tmp/non-existent.config") - _, err := k8s.ListPersistentVolumeClaimsNamesIfConnected(t.Context(), "") + cc, _ := k8s.BuildClientConfig("", "", "", functions.Local{}) + kc := k8s.NewClient(cc) + _, err := k8s.ListPersistentVolumeClaimsNamesIfConnected(t.Context(), kc, "") if err != nil { t.Fatal(err) } diff --git a/pkg/k8s/remover.go b/pkg/k8s/remover.go index 563ef575c2..9eddb3b37d 100644 --- a/pkg/k8s/remover.go +++ b/pkg/k8s/remover.go @@ -10,14 +10,16 @@ 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 { verbose bool + kc *Client } func (remover *Remover) Remove(ctx context.Context, name, ns string) error { @@ -26,7 +28,7 @@ func (remover *Remover) Remove(ctx context.Context, name, ns string) error { return fn.ErrNamespaceRequired } - 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 28c86237b4..13977398e6 100644 --- a/pkg/k8s/remover_int_test.go +++ b/pkg/k8s/remover_int_test.go @@ -10,10 +10,11 @@ import ( ) func TestInt_Remove(t *testing.T) { + kc := defaultKc() removertesting.TestInt_Remove(t, - k8s.NewRemover(true), - k8s.NewDeployer(k8s.WithDeployerVerbose(true)), - k8s.NewDescriber(true), - k8s.NewLister(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..977dd6a8ca 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, kc *Client, name, namespaceOverride string) (*corev1.Secret, error) { + client, namespace, err := kc.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, kc *Client, namespaceOverride string) (names []string, err error) { + names, err = listSecretsNames(ctx, kc, 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, kc *Client, namespaceOverride string) (names []string, err error) { + client, namespace, err := kc.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, kc *Client, namespaceOverride string, listOptions metav1.ListOptions) (err error) { + client, namespace, err := kc.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, kc *Client, name, namespace string, labels, 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, kc, secret, namespace) } -func EnsureSecretExist(ctx context.Context, secret corev1.Secret, namespaceOverride string) (err error) { - client, namespace, err := NewClientAndResolvedNamespace(namespaceOverride) +func EnsureSecretExist(ctx context.Context, kc *Client, secret corev1.Secret, namespaceOverride string) (err error) { + client, namespace, err := kc.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, kc, 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..056d9bf18f 100644 --- a/pkg/k8s/secrets_test.go +++ b/pkg/k8s/secrets_test.go @@ -3,12 +3,15 @@ package k8s_test import ( "testing" + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" ) func TestListSecretsNamesIfConnectedWrongKubeconfig(t *testing.T) { t.Setenv("KUBECONFIG", "/tmp/non-existent.config") - _, err := k8s.ListSecretsNamesIfConnected(t.Context(), "") + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + _, err := k8s.ListSecretsNamesIfConnected(t.Context(), kc, "") if err != nil { t.Fatal(err) } @@ -16,7 +19,9 @@ func TestListSecretsNamesIfConnectedWrongKubeconfig(t *testing.T) { func TestListSecretsNamesIfConnectedWrongKubernentesMaster(t *testing.T) { t.Setenv("KUBERNETES_MASTER", "/tmp/non-existent.config") - _, err := k8s.ListSecretsNamesIfConnected(t.Context(), "") + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + _, err := k8s.ListSecretsNamesIfConnected(t.Context(), kc, "") if err != nil { t.Fatal(err) } diff --git a/pkg/k8s/security_context.go b/pkg/k8s/security_context.go index 4ddaecbf01..8c2b9af89f 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(kc *Client) *corev1.PodSecurityContext { runAsNonRoot := true seccompProfile := &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault} - if IsOpenShift() { + if kc.IsOpenshift() { // 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 246171cd09..26b1b37a28 100644 --- a/pkg/k8s/security_context_test.go +++ b/pkg/k8s/security_context_test.go @@ -6,15 +6,14 @@ 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 testClient(openshift bool) *Client { + return NewClientWithOpenShift(nil, openshift) +} func TestDefaultPodSecurityContext_NonOpenShift(t *testing.T) { - cleanup := SetOpenShiftForTest(false) - defer cleanup() + kc := testClient(false) - sc := defaultPodSecurityContext() + sc := defaultPodSecurityContext(kc) if sc == nil { t.Fatal("expected non-nil PodSecurityContext on non-OpenShift") } @@ -36,10 +35,9 @@ func TestDefaultPodSecurityContext_NonOpenShift(t *testing.T) { } func TestDefaultPodSecurityContext_OpenShift(t *testing.T) { - cleanup := SetOpenShiftForTest(true) - defer cleanup() + kc := testClient(true) - sc := defaultPodSecurityContext() + sc := defaultPodSecurityContext(kc) if sc == nil { t.Fatal("expected non-nil PodSecurityContext on OpenShift") } @@ -52,7 +50,6 @@ func TestDefaultPodSecurityContext_OpenShift(t *testing.T) { if sc.SeccompProfile.Type != corev1.SeccompProfileTypeRuntimeDefault { t.Errorf("expected SeccompProfile.Type=RuntimeDefault, got %v", sc.SeccompProfile.Type) } - // On OpenShift SCCs manage UID/GID; these must not be set. if sc.RunAsUser != nil { t.Errorf("expected RunAsUser to be nil on OpenShift, got %d", *sc.RunAsUser) } @@ -92,13 +89,6 @@ func TestDefaultSecurityContext(t *testing.T) { } } -// TestRestrictedProfileCompliance verifies both security context helpers together -// satisfy all four fields required by the Kubernetes "restricted" pod security -// profile on both OpenShift and vanilla Kubernetes clusters. -// -// Note: this validates Go struct fields only. End-to-end admission validation -// against a real restricted namespace is covered by the integration test suite -// (make test-full). func TestRestrictedProfileCompliance(t *testing.T) { for _, openshift := range []bool{false, true} { openshift := openshift @@ -107,17 +97,14 @@ func TestRestrictedProfileCompliance(t *testing.T) { name = "openshift" } t.Run(name, func(t *testing.T) { - cleanup := SetOpenShiftForTest(openshift) - defer cleanup() + kc := testClient(openshift) - pod := defaultPodSecurityContext() + pod := defaultPodSecurityContext(kc) ctr := defaultSecurityContext() - // restricted requires: allowPrivilegeEscalation=false (container level) if ctr.AllowPrivilegeEscalation == nil || *ctr.AllowPrivilegeEscalation { t.Error("restricted violation: AllowPrivilegeEscalation must be false") } - // restricted requires: capabilities.drop must include ALL (container level) hasDropAll := false if ctr.Capabilities != nil { for _, cap := range ctr.Capabilities.Drop { @@ -130,17 +117,14 @@ func TestRestrictedProfileCompliance(t *testing.T) { if !hasDropAll { t.Error("restricted violation: capabilities.drop must include ALL") } - // restricted requires: runAsNonRoot=true (pod or container level) - podHasRunAsNonRoot := pod != nil && pod.RunAsNonRoot != nil && *pod.RunAsNonRoot - ctrHasRunAsNonRoot := ctr.RunAsNonRoot != nil && *ctr.RunAsNonRoot - if !podHasRunAsNonRoot && !ctrHasRunAsNonRoot { - t.Error("restricted violation: runAsNonRoot must be true at pod or container level") + if pod.RunAsNonRoot == nil || !*pod.RunAsNonRoot { + t.Error("restricted violation: runAsNonRoot must be true") + } + if pod.SeccompProfile == nil { + t.Error("restricted violation: seccompProfile must be set at pod level") } - // restricted requires: seccompProfile (pod or container level) - podHasSeccomp := pod != nil && pod.SeccompProfile != nil - ctrHasSeccomp := ctr.SeccompProfile != nil - if !podHasSeccomp && !ctrHasSeccomp { - t.Error("restricted violation: seccompProfile must be set at pod or container level") + if ctr.SeccompProfile == nil { + t.Error("restricted violation: seccompProfile must be set at container level") } }) } diff --git a/pkg/k8s/serviceaccount.go b/pkg/k8s/serviceaccount.go index 8180cae270..3ef4439ee6 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, kc *Client, referencedServiceAccount, namespace string) error { + k8sClient, err := kc.Clientset() if err != nil { return err } diff --git a/pkg/keda/client.go b/pkg/keda/client.go index b39531281f..bbaa274e56 100644 --- a/pkg/keda/client.go +++ b/pkg/keda/client.go @@ -7,8 +7,8 @@ import ( "knative.dev/func/pkg/k8s" ) -func NewHTTPScaledObjectClientset() (*httpv1alpha1.Clientset, error) { - restConfig, err := k8s.GetClientConfig().ClientConfig() +func NewHTTPScaledObjectClientset(kc *k8s.Client) (*httpv1alpha1.Clientset, error) { + restConfig, err := kc.ClientConfig() 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 17300bfda1..26972d7e00 100644 --- a/pkg/keda/deployer.go +++ b/pkg/keda/deployer.go @@ -29,11 +29,14 @@ type Deployer struct { verbose bool decorator deployer.DeployDecorator + kc *k8s.Client } -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{}), ), @@ -100,7 +103,7 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu // create additional required keda resources namespace := deployResult.Namespace - k8sClientset, err := k8s.NewKubernetesClientset() + k8sClientset, err := d.kc.Clientset() if err != nil { return fn.DeploymentResult{}, fmt.Errorf("failed to create K8sClientset: %v", err) } @@ -265,7 +268,7 @@ func (d *Deployer) ensureHTTPScaledObject(ctx context.Context, f fn.Function, na return fmt.Errorf("failed to generate http scaled object: %w", err) } - httpScaledObjectClientset, err := NewHTTPScaledObjectClientset() + httpScaledObjectClientset, err := NewHTTPScaledObjectClientset(d.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 6e22d09502..ca199049c1 100644 --- a/pkg/keda/deployer_int_test.go +++ b/pkg/keda/deployer_int_test.go @@ -6,72 +6,87 @@ import ( "testing" deployertesting "knative.dev/func/pkg/deployer/testing" + fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" "knative.dev/func/pkg/keda" ) +func defaultKc() *k8s.Client { + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + return k8s.NewClient(cc) +} + func TestInt_FullPath(t *testing.T) { + kc := defaultKc() deployertesting.TestInt_FullPath(t, - keda.NewDeployer(keda.WithDeployerVerbose(false)), - keda.NewRemover(false), - keda.NewLister(false), - keda.NewDescriber(false), + keda.NewDeployer(kc, keda.WithDeployerVerbose(false)), + keda.NewRemover(kc, false), + keda.NewLister(kc, false), + keda.NewDescriber(kc, false), keda.KedaDeployerName) } func TestInt_Deploy(t *testing.T) { + kc := defaultKc() 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 := defaultKc() 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) { t.Skip("Keda deployer does not support func subscribe yet") + kc := defaultKc() 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 := defaultKc() 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 := defaultKc() 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 := defaultKc() 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 := defaultKc() 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/describer.go b/pkg/keda/describer.go index 946f6e2a44..613e3fea2b 100644 --- a/pkg/keda/describer.go +++ b/pkg/keda/describer.go @@ -18,6 +18,7 @@ import ( type Describer struct { verbose bool transport http.RoundTripper + kc *k8s.Client } type DescriberOpt func(*Describer) @@ -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) } @@ -42,7 +43,7 @@ func (d *Describer) Describe(ctx context.Context, name, namespace string) (fn.In return fn.Instance{}, fmt.Errorf("function namespace is required when describing %q", name) } - 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 +67,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..c87bfbbe97 100644 --- a/pkg/keda/describer_int_test.go +++ b/pkg/keda/describer_int_test.go @@ -10,9 +10,10 @@ import ( ) func TestInt_Describe(t *testing.T) { + kc := defaultKc() describertesting.TestInt_Describe(t, - keda.NewDescriber(true), - keda.NewDeployer(keda.WithDeployerVerbose(true)), - keda.NewRemover(true), + keda.NewDescriber(kc, true), + keda.NewDeployer(kc, keda.WithDeployerVerbose(true)), + keda.NewRemover(kc, true), keda.KedaDeployerName) } diff --git a/pkg/keda/lister.go b/pkg/keda/lister.go index 4d100430ae..4569ee0a0e 100644 --- a/pkg/keda/lister.go +++ b/pkg/keda/lister.go @@ -15,21 +15,23 @@ import ( type Lister struct { verbose bool + kc *k8s.Client } -func NewLister(verbose bool) fn.Lister { +func NewLister(kc *k8s.Client, verbose bool) fn.Lister { return &Lister{ + kc: kc, verbose: verbose, } } func (l *Lister) List(ctx context.Context, namespace string) ([]fn.ListItem, error) { - clientset, err := k8s.NewKubernetesClientset() + clientset, err := l.kc.Clientset() if err != nil { return nil, fmt.Errorf("unable to create k8s client: %v", err) } - httpScaledObjectClientset, err := NewHTTPScaledObjectClientset() + httpScaledObjectClientset, err := NewHTTPScaledObjectClientset(l.kc) if err != nil { return nil, fmt.Errorf("unable to create HTTPScaledObject client: %v", err) } diff --git a/pkg/keda/lister_int_test.go b/pkg/keda/lister_int_test.go index 8160086893..c45fcfef04 100644 --- a/pkg/keda/lister_int_test.go +++ b/pkg/keda/lister_int_test.go @@ -10,10 +10,11 @@ import ( ) func TestInt_List(t *testing.T) { + kc := defaultKc() listertesting.TestInt_List(t, - keda.NewLister(true), - keda.NewDeployer(keda.WithDeployerVerbose(true)), - keda.NewDescriber(true), - keda.NewRemover(true), + keda.NewLister(kc, 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 097d020856..b3bc964290 100644 --- a/pkg/keda/remover.go +++ b/pkg/keda/remover.go @@ -11,14 +11,16 @@ import ( "knative.dev/func/pkg/k8s" ) -func NewRemover(verbose bool) *Remover { +func NewRemover(kc *k8s.Client, verbose bool) *Remover { return &Remover{ + kc: kc, verbose: verbose, } } type Remover struct { verbose bool + kc *k8s.Client } func (remover *Remover) Remove(ctx context.Context, name, ns string) error { @@ -27,7 +29,7 @@ func (remover *Remover) Remove(ctx context.Context, name, ns string) error { return fn.ErrNamespaceRequired } - clientset, err := k8s.NewKubernetesClientset() + clientset, err := remover.kc.Clientset() if err != nil { return fmt.Errorf("could not setup kubernetes clientset: %w", err) } diff --git a/pkg/keda/remover_int_test.go b/pkg/keda/remover_int_test.go index 4dfa6e630c..b764e0155e 100644 --- a/pkg/keda/remover_int_test.go +++ b/pkg/keda/remover_int_test.go @@ -10,10 +10,11 @@ import ( ) func TestInt_Remove(t *testing.T) { + kc := defaultKc() removertesting.TestInt_Remove(t, - keda.NewRemover(true), - keda.NewDeployer(keda.WithDeployerVerbose(true)), - keda.NewDescriber(true), - keda.NewLister(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 cfed4a5f8b..5453cbff1e 100644 --- a/pkg/knative/client.go +++ b/pkg/knative/client.go @@ -2,24 +2,17 @@ package knative import ( "fmt" - "os" - "path/filepath" clienteventingv1 "knative.dev/client/pkg/eventing/v1" clientservingv1 "knative.dev/client/pkg/serving/v1" eventingv1 "knative.dev/eventing/pkg/client/clientset/versioned/typed/eventing/v1" servingv1 "knative.dev/serving/pkg/client/clientset/versioned/typed/serving/v1" - fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" ) -func NewServingClient(namespace string) (clientservingv1.KnServingClient, error) { - if err := validateKubeconfigFile(); err != nil { - return nil, err - } - - restConfig, err := k8s.GetClientConfig().ClientConfig() +func NewServingClient(k8sClient *k8s.Client, namespace string) (clientservingv1.KnServingClient, error) { + restConfig, err := k8sClient.ClientConfig() if err != nil { return nil, fmt.Errorf("failed to create new serving client: %v", err) } @@ -34,12 +27,8 @@ func NewServingClient(namespace string) (clientservingv1.KnServingClient, error) return client, nil } -func NewEventingClient(namespace string) (clienteventingv1.KnEventingClient, error) { - if err := validateKubeconfigFile(); err != nil { - return nil, err - } - - restConfig, err := k8s.GetClientConfig().ClientConfig() +func NewEventingClient(k8sClient *k8s.Client, namespace string) (clienteventingv1.KnEventingClient, error) { + restConfig, err := k8sClient.ClientConfig() if err != nil { return nil, fmt.Errorf("failed to create new serving client: %v", err) } @@ -53,26 +42,3 @@ func NewEventingClient(namespace string) (clienteventingv1.KnEventingClient, err return client, nil } - -// validateKubeconfigFile checks if explicitly set KUBECONFIG path exists -func validateKubeconfigFile() error { - kubeconfigPath := os.Getenv("KUBECONFIG") - if kubeconfigPath == "" { - return nil - } - - for _, path := range filepath.SplitList(kubeconfigPath) { - if path == "" { - continue - } - _, statErr := os.Stat(path) - if statErr == nil { - return nil - } - if !os.IsNotExist(statErr) { - return fmt.Errorf("%w: kubeconfig file is not accessible at path: %s", fn.ErrInvalidKubeconfig, path) - } - } - - return fmt.Errorf("%w: kubeconfig file does not exist at path: %s", fn.ErrInvalidKubeconfig, kubeconfigPath) -} diff --git a/pkg/knative/client_test.go b/pkg/knative/client_test.go deleted file mode 100644 index 8564ae6608..0000000000 --- a/pkg/knative/client_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package knative - -import ( - "errors" - "os" - "path/filepath" - "strings" - "testing" - - fn "knative.dev/func/pkg/functions" -) - -func TestValidateKubeconfigFile(t *testing.T) { - testDir := t.TempDir() - existingPath := filepath.Join(testDir, "kubeconfig") - if err := os.WriteFile(existingPath, []byte("apiVersion: v1\n"), 0o600); err != nil { - t.Fatalf("write kubeconfig: %v", err) - } - missingPath := filepath.Join(testDir, "missing") - listSep := string(filepath.ListSeparator) - - cases := []struct { - name string - kubeconfig string - wantErr bool - wantErrText string - }{ - { - name: "empty env", - kubeconfig: "", - wantErr: false, - }, - { - name: "single existing", - kubeconfig: existingPath, - wantErr: false, - }, - { - name: "single missing", - kubeconfig: missingPath, - wantErr: true, - wantErrText: "kubeconfig file does not exist at path", - }, - { - name: "multiple with existing", - kubeconfig: missingPath + listSep + existingPath, - wantErr: false, - }, - { - name: "multiple all missing", - kubeconfig: missingPath + listSep + filepath.Join(testDir, "missing2"), - wantErr: true, - wantErrText: "kubeconfig file does not exist at path", - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Setenv("KUBECONFIG", tc.kubeconfig) - - err := validateKubeconfigFile() - if tc.wantErr { - if err == nil { - t.Fatalf("expected error") - } - if !errors.Is(err, fn.ErrInvalidKubeconfig) { - t.Fatalf("expected ErrInvalidKubeconfig, got: %v", err) - } - if tc.wantErrText != "" && !strings.Contains(err.Error(), tc.wantErrText) { - t.Fatalf("expected error to contain %q, got: %v", tc.wantErrText, err) - } - return - } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - }) - } -} diff --git a/pkg/knative/deployer.go b/pkg/knative/deployer.go index 1bd011d142..0f3a63dfac 100644 --- a/pkg/knative/deployer.go +++ b/pkg/knative/deployer.go @@ -51,11 +51,12 @@ type Deployer struct { // verbose logging enablement flag. verbose bool + k8sClient *k8s.Client decorator deployer.DeployDecorator } -func NewDeployer(opts ...DeployerOpt) *Deployer { - d := &Deployer{} +func NewDeployer(k8sClient *k8s.Client, opts ...DeployerOpt) *Deployer { + d := &Deployer{k8sClient: k8sClient} for _, opt := range opts { opt(d) @@ -83,7 +84,7 @@ func (d *Deployer) isImageInPrivateRegistry(ctx context.Context, client clientse if err != nil { return false } - k8sClient, err := k8s.NewKubernetesClientset() + k8sClient, err := d.k8sClient.Clientset() if err != nil { return false } @@ -106,7 +107,7 @@ func (d *Deployer) isImageInPrivateRegistry(ctx context.Context, client clientse return false } -func onClusterFix(f fn.Function) fn.Function { +func onClusterFix(f fn.Function, kc *k8s.Client) 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 @@ -115,13 +116,13 @@ 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, _ = kc.DefaultNamespace() } return f } func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResult, error) { - f = onClusterFix(f) + f = onClusterFix(f, d.k8sClient) // 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,17 +151,17 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu } // Clients - client, err := NewServingClient(namespace) + client, err := NewServingClient(d.k8sClient, namespace) if err != nil { return fn.DeploymentResult{}, wrapDeployerClientError(err) } - eventingClient, err := NewEventingClient(namespace) + eventingClient, err := NewEventingClient(d.k8sClient, namespace) if err != nil { return fn.DeploymentResult{}, wrapDeployerClientError(err) } // check if 'dapr-system' namespace exists daprInstalled := false - k8sClient, err := k8s.NewKubernetesClientset() + k8sClient, err := d.k8sClient.Clientset() if err != nil { return fn.DeploymentResult{}, wrapDeployerClientError(err) } @@ -169,7 +170,7 @@ 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(d.k8sClient, fnhttp.WithOpenShiftServiceCA(d.k8sClient), fnhttp.WithInsecureSkipVerify(f.RegistryInsecure)) defer func(t fnhttp.RoundTripCloser) { _ = t.Close() }(t) @@ -197,7 +198,7 @@ consider using the --image-pull-secret flag, or setting up pull secrets manually } since := time.Now() go func() { - _ = GetKServiceLogs(ctx, namespace, f.Name, f.Deploy.Image, &since, out) + _ = GetKServiceLogs(ctx, d.k8sClient, namespace, f.Name, f.Deploy.Image, &since, out) }() previousService, err := client.GetService(ctx, f.Name) @@ -217,7 +218,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, d.k8sClient, 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 @@ -319,7 +320,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, d.k8sClient, 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 5a0745a626..dbae1f5a25 100644 --- a/pkg/knative/deployer_int_test.go +++ b/pkg/knative/deployer_int_test.go @@ -6,70 +6,85 @@ import ( "testing" deployertesting "knative.dev/func/pkg/deployer/testing" + fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" "knative.dev/func/pkg/knative" ) +func defaultKc() *k8s.Client { + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + return k8s.NewClient(cc) +} + func TestInt_FullPath(t *testing.T) { + kc := defaultKc() deployertesting.TestInt_FullPath(t, - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewRemover(true), - knative.NewLister(true), - knative.NewDescriber(true), + knative.NewDeployer(kc, knative.WithDeployerVerbose(true)), + knative.NewRemover(kc, true), + knative.NewLister(kc, true), + knative.NewDescriber(kc, true), knative.KnativeDeployerName) } func TestInt_Deploy(t *testing.T) { + kc := defaultKc() 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 := defaultKc() 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 := defaultKc() 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 := defaultKc() 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 := defaultKc() 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 := defaultKc() 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 := defaultKc() 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 37de836cd8..c3e251bbf4 100644 --- a/pkg/knative/describer.go +++ b/pkg/knative/describer.go @@ -18,6 +18,7 @@ import ( type Describer struct { verbose bool + k8sClient *k8s.Client transport http.RoundTripper } @@ -29,8 +30,8 @@ func WithDescriberTransport(transport http.RoundTripper) DescriberOpt { } } -func NewDescriber(verbose bool, opts ...DescriberOpt) *Describer { - d := &Describer{verbose: verbose} +func NewDescriber(k8sClient *k8s.Client, verbose bool, opts ...DescriberOpt) *Describer { + d := &Describer{verbose: verbose, k8sClient: k8sClient} for _, o := range opts { o(d) } @@ -47,12 +48,12 @@ func (d *Describer) Describe(ctx context.Context, name, namespace string) (fn.In return fn.Instance{}, fmt.Errorf("function namespace is required when describing %q", name) } - servingClient, err := NewServingClient(namespace) + servingClient, err := NewServingClient(d.k8sClient, namespace) if err != nil { return fn.Instance{}, err } - eventingClient, err := NewEventingClient(namespace) + eventingClient, err := NewEventingClient(d.k8sClient, namespace) if err != nil { return fn.Instance{}, err } @@ -116,7 +117,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.k8sClient.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..0c4577074e 100644 --- a/pkg/knative/describer_int_test.go +++ b/pkg/knative/describer_int_test.go @@ -10,9 +10,10 @@ import ( ) func TestInt_Describe(t *testing.T) { + kc := defaultKc() 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 ddf86d4534..16e8a9a462 100644 --- a/pkg/knative/lister.go +++ b/pkg/knative/lister.go @@ -7,19 +7,21 @@ import ( "knative.dev/pkg/apis" fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" ) type Lister struct { - verbose bool + verbose bool + k8sClient *k8s.Client } -func NewLister(verbose bool) *Lister { - return &Lister{verbose: verbose} +func NewLister(k8sClient *k8s.Client, verbose bool) *Lister { + return &Lister{verbose: verbose, k8sClient: k8sClient} } // List functions, optionally specifying a namespace. func (l *Lister) List(ctx context.Context, namespace string) ([]fn.ListItem, error) { - client, err := NewServingClient(namespace) + client, err := NewServingClient(l.k8sClient, namespace) if err != nil { return nil, err } diff --git a/pkg/knative/lister_int_test.go b/pkg/knative/lister_int_test.go index a316c55613..9a51cff79e 100644 --- a/pkg/knative/lister_int_test.go +++ b/pkg/knative/lister_int_test.go @@ -10,10 +10,11 @@ import ( ) func TestInt_List(t *testing.T) { + kc := defaultKc() listertesting.TestInt_List(t, - knative.NewLister(true), - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewDescriber(true), - knative.NewRemover(true), + knative.NewLister(kc, 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 19790f7d09..02e3ad3e06 100644 --- a/pkg/knative/logs.go +++ b/pkg/knative/logs.go @@ -16,7 +16,7 @@ import ( // The image must be the digest format since pods of Knative service use it. // // This function runs as long as the passed context is active (i.e. it is required cancel the context to stop log gathering). -func GetKServiceLogs(ctx context.Context, namespace, kServiceName, image string, since *time.Time, out io.Writer) error { +func GetKServiceLogs(ctx context.Context, kc *k8s.Client, namespace, kServiceName, image string, since *time.Time, out io.Writer) error { selector := fmt.Sprintf("serving.knative.dev/service=%s", kServiceName) - return k8s.GetPodLogsBySelector(ctx, namespace, selector, "user-container", image, since, out) + return k8s.GetPodLogsBySelector(ctx, kc, namespace, selector, "user-container", image, since, out) } diff --git a/pkg/knative/remover.go b/pkg/knative/remover.go index ed81738407..7d177e3ded 100644 --- a/pkg/knative/remover.go +++ b/pkg/knative/remover.go @@ -8,18 +8,21 @@ 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(k8sClient *k8s.Client, verbose bool) *Remover { return &Remover{ - verbose: verbose, + verbose: verbose, + k8sClient: k8sClient, } } type Remover struct { - verbose bool + verbose bool + k8sClient *k8s.Client } func (remover *Remover) Remove(ctx context.Context, name, ns string) error { @@ -28,7 +31,7 @@ func (remover *Remover) Remove(ctx context.Context, name, ns string) error { return fn.ErrNamespaceRequired } - client, err := NewServingClient(ns) + client, err := NewServingClient(remover.k8sClient, ns) if err != nil { return err } diff --git a/pkg/knative/remover_int_test.go b/pkg/knative/remover_int_test.go index 6107e96a27..cb95ef798e 100644 --- a/pkg/knative/remover_int_test.go +++ b/pkg/knative/remover_int_test.go @@ -10,10 +10,11 @@ import ( ) func TestInt_Remove(t *testing.T) { + kc := defaultKc() removertesting.TestInt_Remove(t, - knative.NewRemover(true), - knative.NewDeployer(knative.WithDeployerVerbose(true)), - knative.NewDescriber(true), - knative.NewLister(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..8e1a71f427 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, cfg SyncConfig, kc *k8s.Client) error { + restCfg, err := kc.ClientConfig() 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..bbce751cf7 100644 --- a/pkg/operator/syncer.go +++ b/pkg/operator/syncer.go @@ -4,6 +4,7 @@ import ( "context" fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/k8s" "knative.dev/func/pkg/docker" funcgit "knative.dev/func/pkg/git" @@ -14,10 +15,11 @@ type SyncerOpt func(*Syncer) type Syncer struct { credentialsProvider oci.CredentialsProvider + kc *k8s.Client } -func NewSyncer(opts ...SyncerOpt) *Syncer { - s := &Syncer{} +func NewSyncer(kc *k8s.Client, opts ...SyncerOpt) *Syncer { + s := &Syncer{kc: kc} for _, o := range opts { o(s) } @@ -78,5 +80,5 @@ func (s *Syncer) Sync(ctx context.Context, f fn.Function) error { RepoBranch: repoBranch, RepoPath: repoPath, RegistryCredentials: registryCredentials, - }) + }, s.kc) } diff --git a/pkg/pipelines/tekton/client.go b/pkg/pipelines/tekton/client.go index 399ddc35f2..b884a23f6d 100644 --- a/pkg/pipelines/tekton/client.go +++ b/pkg/pipelines/tekton/client.go @@ -14,8 +14,8 @@ const ( ) // NewTektonClient returns TektonV1beta1Client for namespace -func NewTektonClient(namespace string) (*v1.TektonV1Client, error) { - restConfig, err := k8s.GetClientConfig().ClientConfig() +func NewTektonClient(kc *k8s.Client, namespace string) (*v1.TektonV1Client, error) { + restConfig, err := kc.ClientConfig() 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.ClientConfig() 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..9ae0f8833f 100644 --- a/pkg/pipelines/tekton/gitlab_int_test.go +++ b/pkg/pipelines/tekton/gitlab_int_test.go @@ -115,7 +115,9 @@ func TestInt_Gitlab(t *testing.T) { Password: "", }, nil } - pp := tekton.NewPipelinesProvider( + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + pp := tekton.NewPipelinesProvider(kc, tekton.WithCredentialsProvider(credentialsProvider), tekton.WithPacURLCallback(func() (string, error) { return "http://" + pacCtrHostname, nil @@ -613,7 +615,8 @@ func generateSSHKeys(t *testing.T) string { func usingNamespace(t *testing.T) string { name := "gitlab-test-" + strings.ToLower(random.AlphaString(5)) - k8sClient, err := k8s.NewKubernetesClientset() + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + k8sClient, err := k8s.NewClient(cc).Clientset() if err != nil { t.Fatal(err) } @@ -667,7 +670,9 @@ func usingNamespace(t *testing.T) string { func awaitBuildCompletion(t *testing.T, name, ns string) <-chan error { - clis, err := tekton.NewTektonClients() + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + clis, err := tekton.NewTektonClients(kc) if err != nil { t.Fatal(err) } diff --git a/pkg/pipelines/tekton/pac/client.go b/pkg/pipelines/tekton/pac/client.go index 64687d8c17..bfa7905a9d 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.ClientConfig() 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..0e5c1be626 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.ClientAndNamespace("") 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 c0ecf5e87b..105dda298f 100644 --- a/pkg/pipelines/tekton/pipelines_int_test.go +++ b/pkg/pipelines/tekton/pipelines_int_test.go @@ -45,22 +45,24 @@ const ( ) func newRemoteTestClient(verbose bool, deployer string, opts ...fn.Option) *fn.Client { + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) 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.WithListers(knative.NewLister(verbose), k8s.NewLister(verbose), keda.NewLister(verbose)), - fn.WithRemovers(knative.NewRemover(verbose), k8s.NewRemover(verbose), keda.NewRemover(verbose)), - fn.WithPipelinesProvider(tekton.NewPipelinesProvider(tekton.WithCredentialsProvider(testCP), tekton.WithVerbose(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(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...)...) @@ -96,7 +98,9 @@ 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()) + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + dialer, err := k8s.NewInClusterDialer(ctx, kc) if err != nil { t.Fatalf("failed to create in-cluster dialer: %v", err) } @@ -194,7 +198,8 @@ 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() + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + cliSet, err := k8s.NewClient(cc).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..6ac861ebed 100644 --- a/pkg/pipelines/tekton/pipelines_pac_provider.go +++ b/pkg/pipelines/tekton/pipelines_pac_provider.go @@ -63,7 +63,7 @@ func (pp *PipelinesProvider) ConfigurePAC(ctx context.Context, f fn.Function, me data.WebhookSecret = random.AlphaString(10) // try to reuse existing Webhook Secret stored in the cluster - secret, err := k8s.GetSecret(ctx, getPipelineSecretName(f), namespace) + secret, err := k8s.GetSecret(ctx, pp.kc, getPipelineSecretName(f), namespace) if err != nil { if !k8serrors.IsNotFound(err) { return err @@ -157,7 +157,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 +201,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 +228,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 +241,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..b5f76298da 100644 --- a/pkg/pipelines/tekton/pipelines_pac_provider_test.go +++ b/pkg/pipelines/tekton/pipelines_pac_provider_test.go @@ -50,7 +50,7 @@ func Test_createLocalResources(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - pp := NewPipelinesProvider() + pp := NewPipelinesProvider(nil) err = pp.createLocalPACResources(t.Context(), f) if (err != nil) != tt.wantErr { t.Errorf("pp.createLocalResources() error = %v, wantErr %v", err, tt.wantErr) @@ -74,7 +74,7 @@ func Test_deleteAllPipelineTemplates(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - pp := NewPipelinesProvider() + pp := NewPipelinesProvider(nil) 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 fbb700f16b..5dbf160bd9 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,9 @@ func WithPacURLCallback(getPacURL pacURLCallback) Opt { } } -func NewPipelinesProvider(opts ...Opt) *PipelinesProvider { +func NewPipelinesProvider(kc *k8s.Client, opts ...Opt) *PipelinesProvider { pp := &PipelinesProvider{ + kc: kc, getPacURL: func() (string, error) { var url string e := survey.AskOne(&survey.Input{ @@ -151,7 +153,7 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn f.Deploy.Image = image // Client for the given namespace - client, err := NewTektonClient(namespace) + client, err := NewTektonClient(pp.kc, namespace) if err != nil { return "", f, err } @@ -165,7 +167,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 } @@ -174,13 +176,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) { @@ -212,12 +214,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) } @@ -246,19 +248,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) @@ -281,14 +283,17 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn // Creates tar stream with the function sources as they were in "./source" directory. func sourcesAsTarStream(f fn.Function) *io.PipeReader { - ignored := func(p string) bool { return strings.HasPrefix(p, ".git") } - if gi, err := gitignore.CompileIgnoreFile(filepath.Join(f.Root, ".gitignore")); err == nil { - ignored = func(p string) bool { - if strings.HasPrefix(p, ".git") { - return true - } - return gi.MatchesPath(p) + // Apply the same ignore policy as local builds: fn.IsIgnored always excludes + // the DefaultIgnored plus the user's .funcignore patterns. This guarantees local + // runtime data under .func — scaffolding (regenerated on-cluster by the + // func-scaffold step) and any credentials in .func/local.yaml — is never uploaded. + userPatterns := fn.ParseFuncIgnore(f.Root) + gi, giErr := gitignore.CompileIgnoreFile(filepath.Join(f.Root, ".gitignore")) + ignored := func(p string) bool { + if fn.IsIgnored(p, userPatterns) { + return true } + return giErr == nil && gi.MatchesPath(p) } pr, pw := io.Pipe() @@ -327,6 +332,9 @@ func sourcesAsTarStream(f fn.Function) *io.PipeReader { } if ignored(relp) { + if fi.IsDir() { + return filepath.SkipDir + } return nil } @@ -415,7 +423,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, @@ -430,7 +438,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 } @@ -464,7 +472,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 } @@ -511,7 +519,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. @@ -526,7 +534,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 } @@ -579,7 +587,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 != "" { @@ -587,7 +595,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 4d65c99c1e..f7d88057f0 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) { @@ -81,11 +82,60 @@ func TestSourcesAsTarStream(t *testing.T) { } } +// TestSourcesAsTarStream_ExcludesFuncDir ensures the local runtime directory +// (.func) is never uploaded to the cluster — most importantly the credential +// file .func/local.yaml — even when the function has NO .gitignore. The +// exclusion must come from the authoritative DefaultIgnored policy, not from a +// .gitignore happening to list it. Scaffolding under .func/build is regenerated +// on-cluster by the func-scaffold step, so it is not needed in the upload. +func TestSourcesAsTarStream_ExcludesFuncDir(t *testing.T) { + root := t.TempDir() + // deliberately NO .gitignore is written + mustWrite := func(rel, content string) { + p := filepath.Join(root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(p), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), 0600); err != nil { + t.Fatal(err) + } + } + mustWrite("app.js", "module.exports = () => {}\n") // a normal source file + mustWrite(".func/local.yaml", "auth:\n- cluster-url: https://secret\n user:\n token: SECRET\n") + mustWrite(".func/build/service/main.py", "# scaffolding\n") + mustWrite(".func/built-image", "example.com/img@sha256:deadbeef\n") + + rc := sourcesAsTarStream(fn.Function{Root: root}) + t.Cleanup(func() { _ = rc.Close() }) + + var appFound bool + tr := tar.NewReader(rc) + for { + hdr, err := tr.Next() + if err != nil { + if errors.Is(err, io.EOF) { + break + } + t.Fatal(err) + } + if strings.HasPrefix(hdr.Name, "source/.func") { + t.Errorf(".func entry leaked into the upload stream: %q", hdr.Name) + } + if hdr.Name == "source/app.js" { + appFound = true + } + } + if !appFound { + t.Error("the app.js source file is missing from the stream") + } +} + 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, kc *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 + kc *k8s.Client f fn.Function namespace string labels map[string]string @@ -106,7 +156,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, kc *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, @@ -120,7 +170,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, kc *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, @@ -134,20 +184,20 @@ 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, kc *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, }, } for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { // save current function and restore it at the end + t.Run(tt.name, func(t *testing.T) { old := createPersistentVolumeClaim defer func() { createPersistentVolumeClaim = old }() 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, tt.args.kc, 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..86d7ec7a0e 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, namespace) 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, namespace) 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..c0e9ac301e 100644 --- a/pkg/pipelines/tekton/templates.go +++ b/pkg/pipelines/tekton/templates.go @@ -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..8f02cd335c 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(nil, 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..557bfd56c9 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" ) @@ -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..edc1f3df43 100644 --- a/pkg/testing/k8s/testing.go +++ b/pkg/testing/k8s/testing.go @@ -7,6 +7,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/rand" + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" ) @@ -17,7 +18,9 @@ const DefaultIntTestNamespacePrefix = "func-int-test" func Namespace(t *testing.T, ctx context.Context) string { t.Helper() - cliSet, err := k8s.NewKubernetesClientset() + cc, _ := k8s.BuildClientConfig("", "", "", fn.Local{}) + kc := k8s.NewClient(cc) + cliSet, err := kc.Clientset() if err != nil { t.Fatal(err) } diff --git a/schema/func_yaml-schema.json b/schema/func_yaml-schema.json index 47e2e1b2b2..b39500890b 100644 --- a/schema/func_yaml-schema.json +++ b/schema/func_yaml-schema.json @@ -72,6 +72,10 @@ "type": "string", "description": "Namespace into which the function was deployed on supported platforms." }, + "cluster": { + "type": "string", + "description": "Cluster is the cluster server api URL where the function is deployed" + }, "image": { "type": "string", "description": "Image is the deployed image including sha256"