fix: handle incorrect cluster RESTconfig without panic

argoproj · Oct 1, 2024 · 04ac06e · 04ac06e
1 parent 5f23bb6
commit 04ac06e
Show file tree

Hide file tree

Showing 9 changed files with 83 additions and 22 deletions.
diff --git a/cmd/argocd/commands/admin/cluster.go b/cmd/argocd/commands/admin/cluster.go
@@ -565,7 +565,9 @@ argocd admin cluster kubeconfig https://cluster-api-url:6443 /path/to/output/kub
 
 			cluster, err := db.NewDB(namespace, settings.NewSettingsManager(ctx, kubeclientset, namespace), kubeclientset).GetCluster(ctx, serverUrl)
 			errors.CheckError(err)
-			err = kube.WriteKubeConfig(cluster.RawRestConfig(), namespace, output)
+			rawConfig, err := cluster.RawRestConfig()
+			errors.CheckError(err)
+			err = kube.WriteKubeConfig(rawConfig, namespace, output)
 			errors.CheckError(err)
 		},
 	}

diff --git a/controller/appcontroller.go b/controller/appcontroller.go
@@ -1162,7 +1162,11 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
 		logCtx.Infof("Resource entries removed from undefined cluster")
 		return nil
 	}
-	config := metrics.AddMetricsTransportWrapper(ctrl.metricsServer, app, cluster.RESTConfig())
+	clusterRESTConfig, err := cluster.RESTConfig()
+	if err != nil {
+		return err
+	}
+	config := metrics.AddMetricsTransportWrapper(ctrl.metricsServer, app, clusterRESTConfig)
 
 	if app.CascadedDeletion() {
 		logCtx.Infof("Deleting resources")

diff --git a/controller/cache/cache.go b/controller/cache/cache.go
@@ -490,7 +490,10 @@ func (c *liveStateCache) getCluster(server string) (clustercache.ClusterCache, e
 		return nil, fmt.Errorf("error getting value for %v: %w", settings.RespectRBAC, err)
 	}
 
-	clusterCacheConfig := cluster.RESTConfig()
+	clusterCacheConfig, err := cluster.RESTConfig()
+	if err != nil {
+		return nil, fmt.Errorf("error getting cluster RESTConfig: %w", err)
+	}
 	// Controller dynamically fetches all resource types available on the cluster
 	// using a discovery API that may contain deprecated APIs.
 	// This causes log flooding when managing a large number of clusters.
@@ -821,7 +824,12 @@ func (c *liveStateCache) handleModEvent(oldCluster *appv1.Cluster, newCluster *a
 
 		var updateSettings []clustercache.UpdateSettingsFunc
 		if !reflect.DeepEqual(oldCluster.Config, newCluster.Config) {
-			updateSettings = append(updateSettings, clustercache.SetConfig(newCluster.RESTConfig()))
+			newClusterRESTConfig, err := newCluster.RESTConfig()
+			if err == nil {
+				updateSettings = append(updateSettings, clustercache.SetConfig(newClusterRESTConfig))
+			} else {
+				log.Errorf("error getting cluster REST config: %v", err)
+			}
 		}
 		if !reflect.DeepEqual(oldCluster.Namespaces, newCluster.Namespaces) {
 			updateSettings = append(updateSettings, clustercache.SetNamespaces(newCluster.Namespaces))

diff --git a/controller/sync.go b/controller/sync.go
@@ -76,7 +76,12 @@ func (m *appStateManager) getResourceOperations(server string) (kube.ResourceOpe
 	if err != nil {
 		return nil, nil, fmt.Errorf("error getting cluster: %w", err)
 	}
-	ops, cleanup, err := m.kubectl.ManageResources(cluster.RawRestConfig(), clusterCache.GetOpenAPISchema())
+
+	rawConfig, err := cluster.RawRestConfig()
+	if err != nil {
+		return nil, nil, fmt.Errorf("error getting cluster REST config: %w", err)
+	}
+	ops, cleanup, err := m.kubectl.ManageResources(rawConfig, clusterCache.GetOpenAPISchema())
 	if err != nil {
 		return nil, nil, fmt.Errorf("error creating kubectl ResourceOperations: %w", err)
 	}
@@ -213,8 +218,20 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
 		return
 	}
 
-	rawConfig := clst.RawRestConfig()
-	restConfig := metrics.AddMetricsTransportWrapper(m.metricsServer, app, clst.RESTConfig())
+	rawConfig, err := clst.RawRestConfig()
+	if err != nil {
+		state.Phase = common.OperationError
+		state.Message = err.Error()
+		return
+	}
+
+	clusterRESTConfig, err := clst.RESTConfig()
+	if err != nil {
+		state.Phase = common.OperationError
+		state.Message = err.Error()
+		return
+	}
+	restConfig := metrics.AddMetricsTransportWrapper(m.metricsServer, app, clusterRESTConfig)
 
 	resourceOverrides, err := m.settingsMgr.GetResourceOverrides()
 	if err != nil {

diff --git a/pkg/apis/application/v1alpha1/types.go b/pkg/apis/application/v1alpha1/types.go
@@ -3098,7 +3098,7 @@ func SetK8SConfigDefaults(config *rest.Config) error {
 }
 
 // RawRestConfig returns a go-client REST config from cluster that might be serialized into the file using kube.WriteKubeConfig method.
-func (c *Cluster) RawRestConfig() *rest.Config {
+func (c *Cluster) RawRestConfig() (*rest.Config, error) {
 	var config *rest.Config
 	var err error
 	if c.Server == KubernetesInternalAPIServerAddr && env.ParseBoolFromEnv(EnvVarFakeInClusterConfig, false) {
@@ -3182,22 +3182,25 @@ func (c *Cluster) RawRestConfig() *rest.Config {
 		}
 	}
 	if err != nil {
-		panic(fmt.Sprintf("Unable to create K8s REST config: %v", err))
+		return nil, fmt.Errorf("Unable to create K8s REST config: %w", err)
 	}
 	config.Timeout = K8sServerSideTimeout
 	config.QPS = K8sClientConfigQPS
 	config.Burst = K8sClientConfigBurst
-	return config
+	return config, nil
 }
 
 // RESTConfig returns a go-client REST config from cluster with tuned throttling and HTTP client settings.
-func (c *Cluster) RESTConfig() *rest.Config {
-	config := c.RawRestConfig()
-	err := SetK8SConfigDefaults(config)
+func (c *Cluster) RESTConfig() (*rest.Config, error) {
+	config, err := c.RawRestConfig()
 	if err != nil {
-		panic(fmt.Sprintf("Unable to apply K8s REST config defaults: %v", err))
+		return nil, fmt.Errorf("Unable to get K8s RAW REST config: %w", err)
 	}
-	return config
+	err = SetK8SConfigDefaults(config)
+	if err != nil {
+		return nil, fmt.Errorf("Unable to apply K8s REST config defaults: %w", err)
+	}
+	return config, nil
 }
 
 // UnmarshalToUnstructured unmarshals a resource representation in JSON to unstructured data

diff --git a/server/application/application.go b/server/application/application.go
@@ -1281,7 +1281,11 @@ func (s *Server) getApplicationClusterConfig(ctx context.Context, a *appv1.Appli
 	if err != nil {
 		return nil, fmt.Errorf("error getting cluster: %w", err)
 	}
-	config := clst.RESTConfig()
+	config, err := clst.RESTConfig()
+	if err != nil {
+		return nil, fmt.Errorf("error getting cluster REST config: %w", err)
+	}
+
 	return config, err
 }
 

diff --git a/server/application/terminal.go b/server/application/terminal.go
@@ -70,7 +70,11 @@ func (s *terminalHandler) getApplicationClusterRawConfig(ctx context.Context, a
 	if err != nil {
 		return nil, err
 	}
-	return clst.RawRestConfig(), nil
+	rawConfig, err := clst.RawRestConfig()
+	if err != nil {
+		return nil, err
+	}
+	return rawConfig, nil
 }
 
 type GetSettingsFunc func() (*settings.ArgoCDSettings, error)

diff --git a/server/cluster/cluster.go b/server/cluster/cluster.go
@@ -146,7 +146,12 @@ func (s *Server) Create(ctx context.Context, q *cluster.ClusterCreateRequest) (*
 		return nil, err
 	}
 	c := q.Cluster
-	serverVersion, err := s.kubectl.GetServerVersion(c.RESTConfig())
+	clusterRESTConfig, err := c.RESTConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	serverVersion, err := s.kubectl.GetServerVersion(clusterRESTConfig)
 	if err != nil {
 		return nil, err
 	}
@@ -313,9 +318,13 @@ func (s *Server) Update(ctx context.Context, q *cluster.ClusterUpdateRequest) (*
 		}
 		q.Cluster = c
 	}
+	clusterRESTConfig, err := q.Cluster.RESTConfig()
+	if err != nil {
+		return nil, err
+	}
 
 	// Test the token we just created before persisting it
-	serverVersion, err := s.kubectl.GetServerVersion(q.Cluster.RESTConfig())
+	serverVersion, err := s.kubectl.GetServerVersion(clusterRESTConfig)
 	if err != nil {
 		return nil, err
 	}
@@ -406,7 +415,10 @@ func (s *Server) RotateAuth(ctx context.Context, q *cluster.ClusterQuery) (*clus
 	for _, server := range servers {
 		logCtx := log.WithField("cluster", server)
 		logCtx.Info("Rotating auth")
-		restCfg := clust.RESTConfig()
+		restCfg, err := clust.RESTConfig()
+		if err != nil {
+			return nil, err
+		}
 		if restCfg.BearerToken == "" {
 			return nil, status.Errorf(codes.InvalidArgument, "Cluster '%s' does not use bearer token authentication", server)
 		}
@@ -428,8 +440,12 @@ func (s *Server) RotateAuth(ctx context.Context, q *cluster.ClusterQuery) (*clus
 		clust.Config.CertData = nil
 		clust.Config.BearerToken = string(newSecret.Data["token"])
 
+		clusterRESTConfig, err := clust.RESTConfig()
+		if err != nil {
+			return nil, err
+		}
 		// Test the token we just created before persisting it
-		serverVersion, err := s.kubectl.GetServerVersion(clust.RESTConfig())
+		serverVersion, err := s.kubectl.GetServerVersion(clusterRESTConfig)
 		if err != nil {
 			return nil, err
 		}

diff --git a/util/argo/argo.go b/util/argo/argo.go
@@ -329,7 +329,10 @@ func ValidateRepo(
 		})
 		return conditions, nil
 	}
-	config := cluster.RESTConfig()
+	config, err := cluster.RESTConfig()
+	if err != nil {
+		return nil, fmt.Errorf("error getting cluster REST config: %w", err)
+	}
 	// nolint:staticcheck
 	cluster.ServerVersion, err = kubectl.GetServerVersion(config)
 	if err != nil {