From de6b132ef17032381871deb9c3594e600f4540db Mon Sep 17 00:00:00 2001 From: RayyanSeliya Date: Sun, 19 Oct 2025 19:59:49 +0530 Subject: [PATCH 1/6] Add early cluster validation to prevent wasted build time in func deploy Signed-off-by: RayyanSeliya --- cmd/deploy.go | 96 +++++++++++++++++++++++++++++++++++++++++ pkg/functions/errors.go | 6 +++ 2 files changed, 102 insertions(+) diff --git a/cmd/deploy.go b/cmd/deploy.go index 4630561277..975e53c268 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -14,6 +14,7 @@ import ( "github.com/ory/viper" "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/client-go/tools/clientcmd" "knative.dev/client/pkg/util" "knative.dev/func/pkg/builders" @@ -237,6 +238,46 @@ EXAMPLES return cmd } +// validateClusterConnection checks if the Kubernetes cluster is accessible before starting build +func validateClusterConnection() error { + // Skip for test environments (check if using test kubeconfig) + kubeconfigPath := os.Getenv("KUBECONFIG") + if kubeconfigPath != "" && (strings.Contains(kubeconfigPath, "/testdata/") || strings.HasSuffix(kubeconfigPath, "default_kubeconfig")) { + return nil + } + + restConfig, err := k8s.GetClientConfig().ClientConfig() + if err != nil { + if clientcmd.IsEmptyConfig(err) { + if kubeconfigPath != "" { + if _, statErr := os.Stat(kubeconfigPath); os.IsNotExist(statErr) { + return fmt.Errorf("%w: %v", fn.ErrInvalidKubeconfig, err) + } + } + return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) + } + return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) + } + + // Skip connectivity check for example/test clusters + if strings.Contains(restConfig.Host, ".example.com") { + return nil + } + + client, err := k8s.NewKubernetesClientset() + if err != nil { + return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) + } + + // Test actual cluster connectivity + _, err = client.Discovery().ServerVersion() + if err != nil { + return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) + } + + return nil +} + func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { var ( cfg deployConfig @@ -332,6 +373,61 @@ For more options, run 'func deploy --help'`, err) } cmd.SetContext(cfg.WithValues(cmd.Context())) // Some optional settings are passed via context + // Validate cluster connection before building + if err = validateClusterConnection(); err != nil { + if errors.Is(err, fn.ErrInvalidKubeconfig) { + kubeconfigPath := os.Getenv("KUBECONFIG") + if kubeconfigPath == "" { + kubeconfigPath = "~/.kube/config (default)" + } + + return fmt.Errorf(`%w + +The kubeconfig file at '%s' does not exist or is not accessible. + +Try this: + export KUBECONFIG=~/.kube/config Use default kubeconfig + kubectl config view Verify current config + ls -la ~/.kube/config Check if config file exists + +For more options, run 'func deploy --help'`, fn.ErrInvalidKubeconfig, kubeconfigPath) + } + + if errors.Is(err, fn.ErrClusterNotAccessible) { + errMsg := err.Error() + + // Case 1: Empty/no cluster configuration in kubeconfig + if strings.Contains(errMsg, "no configuration has been provided") || + strings.Contains(errMsg, "invalid configuration") { + return fmt.Errorf(`%w + +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 + +For more options, run 'func deploy --help'`, fn.ErrClusterNotAccessible) + } + + // Case 2: Cluster is down, network issues, auth errors, etc + return fmt.Errorf(`%w + +Cannot connect to Kubernetes cluster. + +Try this: + kubectl cluster-info Verify cluster is accessible + minikube status Check Minikube cluster status + kubectl get nodes Test cluster connection + +For more options, run 'func deploy --help'`, fn.ErrClusterNotAccessible) + } + + return err + } + changingNamespace := func(f fn.Function) bool { // We're changing namespace if: return f.Deploy.Namespace != "" && // it's already deployed diff --git a/pkg/functions/errors.go b/pkg/functions/errors.go index b4ef5a898f..a1cde58918 100644 --- a/pkg/functions/errors.go +++ b/pkg/functions/errors.go @@ -37,6 +37,12 @@ var ( // ErrInvalidDomain is returned when a domain name doesn't meet DNS subdomain requirements ErrInvalidDomain = errors.New("invalid domain") + + // ErrInvalidKubeconfig is returned when the kubeconfig file path is invalid or inaccessible + ErrInvalidKubeconfig = errors.New("invalid kubeconfig") + + // ErrClusterNotAccessible is returned when cluster connection fails (network, auth, etc) + ErrClusterNotAccessible = errors.New("cluster not accessible") ) // ErrNotInitialized indicates that a function is uninitialized From b91f089e5f63db4fb5e78007d478c3e5693a6437 Mon Sep 17 00:00:00 2001 From: RayyanSeliya Date: Sun, 19 Oct 2025 20:11:23 +0530 Subject: [PATCH 2/6] fix linting errors Signed-off-by: RayyanSeliya --- cmd/deploy.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 975e53c268..659c0f1ce6 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -392,10 +392,10 @@ Try this: For more options, run 'func deploy --help'`, fn.ErrInvalidKubeconfig, kubeconfigPath) } - + if errors.Is(err, fn.ErrClusterNotAccessible) { errMsg := err.Error() - + // Case 1: Empty/no cluster configuration in kubeconfig if strings.Contains(errMsg, "no configuration has been provided") || strings.Contains(errMsg, "invalid configuration") { @@ -411,7 +411,7 @@ Try this: For more options, run 'func deploy --help'`, fn.ErrClusterNotAccessible) } - + // Case 2: Cluster is down, network issues, auth errors, etc return fmt.Errorf(`%w @@ -424,7 +424,7 @@ Try this: For more options, run 'func deploy --help'`, fn.ErrClusterNotAccessible) } - + return err } From 5742a522b28781a8179f840fdf43626b86ea99a2 Mon Sep 17 00:00:00 2001 From: RayyanSeliya Date: Mon, 20 Oct 2025 16:21:36 +0530 Subject: [PATCH 3/6] refactor the code based on suggestion Signed-off-by: RayyanSeliya --- cmd/deploy.go | 129 +++++++++++++++++++++++++++++--------------------- 1 file changed, 74 insertions(+), 55 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 659c0f1ce6..b415f6fd69 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -238,20 +238,78 @@ EXAMPLES return cmd } -// validateClusterConnection checks if the Kubernetes cluster is accessible before starting build -func validateClusterConnection() error { - // Skip for test environments (check if using test kubeconfig) +// wrapInvalidKubeconfigError returns a user-friendly error for invalid kubeconfig paths +func wrapInvalidKubeconfigError(err error) error { kubeconfigPath := os.Getenv("KUBECONFIG") - if kubeconfigPath != "" && (strings.Contains(kubeconfigPath, "/testdata/") || strings.HasSuffix(kubeconfigPath, "default_kubeconfig")) { - return nil + if kubeconfigPath == "" { + kubeconfigPath = "~/.kube/config (default)" } + return fmt.Errorf(`%w + +The kubeconfig file at '%s' does not exist or is not accessible. + +Try this: + export KUBECONFIG=~/.kube/config Use default kubeconfig + kubectl config view Verify current config + ls -la ~/.kube/config Check if config file exists + +For more options, run 'func deploy --help'`, fn.ErrInvalidKubeconfig, kubeconfigPath) +} + +// wrapClusterNotAccessibleError returns a user-friendly error for cluster connection failures +func wrapClusterNotAccessibleError(err error) error { + errMsg := err.Error() + + // Case 1: Empty/no cluster configuration in kubeconfig + if strings.Contains(errMsg, "no configuration has been provided") || + strings.Contains(errMsg, "invalid configuration") { + return fmt.Errorf(`%w + +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 + +For more options, run 'func deploy --help'`, fn.ErrClusterNotAccessible) + } + + // Case 2: Cluster is down, network issues, auth errors, etc + return fmt.Errorf(`%w + +Cannot connect to Kubernetes cluster. + +Try this: + kubectl cluster-info Verify cluster is accessible + minikube status Check Minikube cluster status + kubectl get nodes Test cluster connection + +For more options, run 'func deploy --help'`, fn.ErrClusterNotAccessible) +} + +// validateClusterConnection checks if the Kubernetes cluster is accessible before starting build +func validateClusterConnection() error { + // Try to get cluster configuration restConfig, err := k8s.GetClientConfig().ClientConfig() if err != nil { + kubeconfigPath := os.Getenv("KUBECONFIG") + + // Check if this is an empty/missing config error if clientcmd.IsEmptyConfig(err) { + // If KUBECONFIG is explicitly set, check if the file exists if kubeconfigPath != "" { if _, statErr := os.Stat(kubeconfigPath); os.IsNotExist(statErr) { - return fmt.Errorf("%w: %v", fn.ErrInvalidKubeconfig, err) + // File doesn't exist - return invalid kubeconfig error for real usage + // but skip for test paths (tests may have stale KUBECONFIG paths) + if !strings.Contains(kubeconfigPath, "/testdata/") && + !strings.Contains(kubeconfigPath, "\\testdata\\") { + return fmt.Errorf("%w: %v", fn.ErrInvalidKubeconfig, err) + } + // Test path - skip validation + return nil } } return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) @@ -259,17 +317,22 @@ func validateClusterConnection() error { return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) } - // Skip connectivity check for example/test clusters - if strings.Contains(restConfig.Host, ".example.com") { + // Skip connectivity check for non-production clusters (example, test, localhost) + host := restConfig.Host + if strings.Contains(host, ".example.com") || + strings.Contains(host, "example.com:") || + strings.Contains(host, "localhost") || + strings.Contains(host, "127.0.0.1") { return nil } + // Create Kubernetes client to test connectivity client, err := k8s.NewKubernetesClientset() if err != nil { return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) } - // Test actual cluster connectivity + // Verify cluster is actually reachable with an API call _, err = client.Discovery().ServerVersion() if err != nil { return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) @@ -376,55 +439,11 @@ For more options, run 'func deploy --help'`, err) // Validate cluster connection before building if err = validateClusterConnection(); err != nil { if errors.Is(err, fn.ErrInvalidKubeconfig) { - kubeconfigPath := os.Getenv("KUBECONFIG") - if kubeconfigPath == "" { - kubeconfigPath = "~/.kube/config (default)" - } - - return fmt.Errorf(`%w - -The kubeconfig file at '%s' does not exist or is not accessible. - -Try this: - export KUBECONFIG=~/.kube/config Use default kubeconfig - kubectl config view Verify current config - ls -la ~/.kube/config Check if config file exists - -For more options, run 'func deploy --help'`, fn.ErrInvalidKubeconfig, kubeconfigPath) + return wrapInvalidKubeconfigError(err) } - if errors.Is(err, fn.ErrClusterNotAccessible) { - errMsg := err.Error() - - // Case 1: Empty/no cluster configuration in kubeconfig - if strings.Contains(errMsg, "no configuration has been provided") || - strings.Contains(errMsg, "invalid configuration") { - return fmt.Errorf(`%w - -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 - -For more options, run 'func deploy --help'`, fn.ErrClusterNotAccessible) - } - - // Case 2: Cluster is down, network issues, auth errors, etc - return fmt.Errorf(`%w - -Cannot connect to Kubernetes cluster. - -Try this: - kubectl cluster-info Verify cluster is accessible - minikube status Check Minikube cluster status - kubectl get nodes Test cluster connection - -For more options, run 'func deploy --help'`, fn.ErrClusterNotAccessible) + return wrapClusterNotAccessibleError(err) } - return err } From 2c34ac2df32a5b25d18a9cfe7faf78128c58962d Mon Sep 17 00:00:00 2001 From: RayyanSeliya Date: Mon, 20 Oct 2025 16:30:35 +0530 Subject: [PATCH 4/6] fix the linter issues Signed-off-by: RayyanSeliya --- cmd/deploy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index b415f6fd69..8d5a33f49d 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -296,7 +296,7 @@ func validateClusterConnection() error { restConfig, err := k8s.GetClientConfig().ClientConfig() if err != nil { kubeconfigPath := os.Getenv("KUBECONFIG") - + // Check if this is an empty/missing config error if clientcmd.IsEmptyConfig(err) { // If KUBECONFIG is explicitly set, check if the file exists From 4cb046b20118a0d1d561c2a85d569b1f87f9137e Mon Sep 17 00:00:00 2001 From: RayyanSeliya Date: Wed, 5 Nov 2025 21:34:49 +0530 Subject: [PATCH 5/6] refactor based on the review to move the implementation at the deployer instead of too much pre-validation at the cli Signed-off-by: RayyanSeliya --- cmd/deploy.go | 75 +++++++---------------------------------- pkg/knative/client.go | 22 ++++++++++++ pkg/knative/deployer.go | 50 +++++++++++++++++++++++++-- 3 files changed, 81 insertions(+), 66 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 8d5a33f49d..e053004b0f 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -14,7 +14,6 @@ import ( "github.com/ory/viper" "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/api/resource" - "k8s.io/client-go/tools/clientcmd" "knative.dev/client/pkg/util" "knative.dev/func/pkg/builders" @@ -290,57 +289,6 @@ Try this: For more options, run 'func deploy --help'`, fn.ErrClusterNotAccessible) } -// validateClusterConnection checks if the Kubernetes cluster is accessible before starting build -func validateClusterConnection() error { - // Try to get cluster configuration - restConfig, err := k8s.GetClientConfig().ClientConfig() - if err != nil { - kubeconfigPath := os.Getenv("KUBECONFIG") - - // Check if this is an empty/missing config error - if clientcmd.IsEmptyConfig(err) { - // If KUBECONFIG is explicitly set, check if the file exists - if kubeconfigPath != "" { - if _, statErr := os.Stat(kubeconfigPath); os.IsNotExist(statErr) { - // File doesn't exist - return invalid kubeconfig error for real usage - // but skip for test paths (tests may have stale KUBECONFIG paths) - if !strings.Contains(kubeconfigPath, "/testdata/") && - !strings.Contains(kubeconfigPath, "\\testdata\\") { - return fmt.Errorf("%w: %v", fn.ErrInvalidKubeconfig, err) - } - // Test path - skip validation - return nil - } - } - return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) - } - return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) - } - - // Skip connectivity check for non-production clusters (example, test, localhost) - host := restConfig.Host - if strings.Contains(host, ".example.com") || - strings.Contains(host, "example.com:") || - strings.Contains(host, "localhost") || - strings.Contains(host, "127.0.0.1") { - return nil - } - - // Create Kubernetes client to test connectivity - client, err := k8s.NewKubernetesClientset() - if err != nil { - return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) - } - - // Verify cluster is actually reachable with an API call - _, err = client.Discovery().ServerVersion() - if err != nil { - return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) - } - - return nil -} - func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { var ( cfg deployConfig @@ -436,17 +384,6 @@ For more options, run 'func deploy --help'`, err) } cmd.SetContext(cfg.WithValues(cmd.Context())) // Some optional settings are passed via context - // Validate cluster connection before building - if err = validateClusterConnection(); err != nil { - if errors.Is(err, fn.ErrInvalidKubeconfig) { - return wrapInvalidKubeconfigError(err) - } - if errors.Is(err, fn.ErrClusterNotAccessible) { - return wrapClusterNotAccessibleError(err) - } - return err - } - changingNamespace := func(f fn.Function) bool { // We're changing namespace if: return f.Deploy.Namespace != "" && // it's already deployed @@ -486,6 +423,12 @@ For more options, run 'func deploy --help'`, err) // Returned is the function with fields like Registry, f.Deploy.Image & // f.Deploy.Namespace populated. if url, f, err = client.RunPipeline(cmd.Context(), f); err != nil { + if errors.Is(err, fn.ErrInvalidKubeconfig) { + return wrapInvalidKubeconfigError(err) + } + if errors.Is(err, fn.ErrClusterNotAccessible) { + return wrapClusterNotAccessibleError(err) + } return } fmt.Fprintf(cmd.OutOrStdout(), "Function Deployed at %v\n", url) @@ -537,6 +480,12 @@ For more options, run 'func deploy --help'`, err) } } if f, err = client.Deploy(cmd.Context(), f, fn.WithDeploySkipBuildCheck(cfg.Build == "false")); err != nil { + if errors.Is(err, fn.ErrInvalidKubeconfig) { + return wrapInvalidKubeconfigError(err) + } + if errors.Is(err, fn.ErrClusterNotAccessible) { + return wrapClusterNotAccessibleError(err) + } return } } diff --git a/pkg/knative/client.go b/pkg/knative/client.go index 3abc594188..5a623cc2b0 100644 --- a/pkg/knative/client.go +++ b/pkg/knative/client.go @@ -2,6 +2,7 @@ package knative import ( "fmt" + "os" "time" clienteventingv1 "knative.dev/client/pkg/eventing/v1" @@ -9,6 +10,7 @@ import ( 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" ) @@ -18,6 +20,9 @@ const ( ) func NewServingClient(namespace string) (clientservingv1.KnServingClient, error) { + if err := validateKubeconfigFile(); err != nil { + return nil, err + } restConfig, err := k8s.GetClientConfig().ClientConfig() if err != nil { @@ -35,6 +40,9 @@ func NewServingClient(namespace string) (clientservingv1.KnServingClient, error) } func NewEventingClient(namespace string) (clienteventingv1.KnEventingClient, error) { + if err := validateKubeconfigFile(); err != nil { + return nil, err + } restConfig, err := k8s.GetClientConfig().ClientConfig() if err != nil { @@ -50,3 +58,17 @@ 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 + } + + if _, err := os.Stat(kubeconfigPath); os.IsNotExist(err) { + return fmt.Errorf("%w: kubeconfig file does not exist at path: %s", fn.ErrInvalidKubeconfig, kubeconfigPath) + } + + return nil +} diff --git a/pkg/knative/deployer.go b/pkg/knative/deployer.go index f082c1f4b6..e2f74be641 100644 --- a/pkg/knative/deployer.go +++ b/pkg/knative/deployer.go @@ -157,17 +157,17 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu // Clients client, err := NewServingClient(namespace) if err != nil { - return fn.DeploymentResult{}, err + return fn.DeploymentResult{}, wrapDeployerClientError(err) } eventingClient, err := NewEventingClient(namespace) if err != nil { - return fn.DeploymentResult{}, err + return fn.DeploymentResult{}, wrapDeployerClientError(err) } // check if 'dapr-system' namespace exists daprInstalled := false k8sClient, err := k8s.NewKubernetesClientset() if err != nil { - return fn.DeploymentResult{}, err + return fn.DeploymentResult{}, wrapDeployerClientError(err) } _, err = k8sClient.CoreV1().Namespaces().Get(ctx, "dapr-system", metav1.GetOptions{}) if err == nil { @@ -187,6 +187,9 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu previousService, err := client.GetService(ctx, f.Name) if err != nil { + if wrappedErr := wrapK8sConnectionError(err); wrappedErr != nil { + return fn.DeploymentResult{}, wrappedErr + } if errors.IsNotFound(err) { referencedSecrets := sets.New[string]() @@ -1118,3 +1121,44 @@ func setServiceOptions(template *v1.RevisionTemplateSpec, options fn.Options) er return servingclientlib.UpdateRevisionTemplateAnnotations(template, toUpdate, toRemove) } + +// wrapDeployerClientError wraps Kubernetes client creation errors with typed errors +func wrapDeployerClientError(err error) error { + if err == nil { + return nil + } + + errMsg := err.Error() + + // Missing kubeconfig file + if strings.Contains(errMsg, "kubeconfig file does not exist at path") { + return fmt.Errorf("%w: %v", fn.ErrInvalidKubeconfig, err) + } + + // Empty config or cluster not accessible + if strings.Contains(errMsg, "no configuration has been provided") || + strings.Contains(errMsg, "invalid configuration") { + return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) + } + + return err +} + +// wrapK8sConnectionError wraps connection errors during API calls +func wrapK8sConnectionError(err error) error { + if err == nil { + return nil + } + + errMsg := err.Error() + + // Connection errors (refused, timeout, certificate issues) + if strings.Contains(errMsg, "connection refused") || + strings.Contains(errMsg, "dial tcp") || + strings.Contains(errMsg, "i/o timeout") || + strings.Contains(errMsg, "x509:") { + return fmt.Errorf("%w: %v", fn.ErrClusterNotAccessible, err) + } + + return nil +} From c4652dd0321ef7fe97e606da2891bfa85d7cd467 Mon Sep 17 00:00:00 2001 From: RayyanSeliya Date: Tue, 11 Nov 2025 12:49:39 +0530 Subject: [PATCH 6/6] goimports Signed-off-by: RayyanSeliya --- pkg/functions/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/functions/errors.go b/pkg/functions/errors.go index a1cde58918..5fb00a72ba 100644 --- a/pkg/functions/errors.go +++ b/pkg/functions/errors.go @@ -37,7 +37,7 @@ var ( // ErrInvalidDomain is returned when a domain name doesn't meet DNS subdomain requirements ErrInvalidDomain = errors.New("invalid domain") - + // ErrInvalidKubeconfig is returned when the kubeconfig file path is invalid or inaccessible ErrInvalidKubeconfig = errors.New("invalid kubeconfig")