From 03f540606f96ec44ec6552cb8e2197f24d5ac59e Mon Sep 17 00:00:00 2001 From: Michael Nelson Date: Thu, 14 Apr 2022 07:16:02 +1000 Subject: [PATCH 1/2] Use an explicit token to identify kubeapps cluster when required. For discussion. Signed-off-by: Michael Nelson --- .../core/plugins/v1alpha1/plugins.go | 13 +++++----- .../plugins/helm/packages/v1alpha1/server.go | 5 ---- pkg/kube/kube_handler.go | 25 +++++++++++++++---- pkg/kube/kube_handler_test.go | 6 ++--- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/cmd/kubeapps-apis/core/plugins/v1alpha1/plugins.go b/cmd/kubeapps-apis/core/plugins/v1alpha1/plugins.go index 8f45de71583..124417fab0e 100644 --- a/cmd/kubeapps-apis/core/plugins/v1alpha1/plugins.go +++ b/cmd/kubeapps-apis/core/plugins/v1alpha1/plugins.go @@ -323,13 +323,12 @@ func createConfigGetterWithParams(inClusterConfig *rest.Config, serveOpts core.S var config *rest.Config - // Enable existing plugins to pass an empty cluster name to get the - // kubeapps cluster for now, until we support (or otherwise decide) - // multicluster configuration of all plugins. - if cluster == "" { - cluster = clustersConfig.KubeappsClusterName - } - + // Plugins are now required to define the complete context for any + // package, including the cluster and namespace, though there is one + // scenario where the cluster name cannot be specified: when Kubeapps is + // configured without the cluster on which Kubeapps is installed in the + // cluster configuration. In this case, global charts cannot specify a + // name for the cluster. config, err = kube.NewClusterConfig(inClusterConfig, token, cluster, clustersConfig) if err != nil { return nil, fmt.Errorf("unable to get clusterConfig: %w", err) diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go index 1f6d10c75f1..405ea642453 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go @@ -145,11 +145,6 @@ func NewServer(configGetter core.KubernetesConfigGetter, globalPackagingCluster clientGetter: clientgetter.NewClientGetter(configGetter, clientgetter.Options{}), actionConfigGetter: func(ctx context.Context, pkgContext *corev1.Context) (*action.Configuration, error) { cluster := pkgContext.GetCluster() - // Don't force clients to send a cluster unless we are sure all use-cases - // of kubeapps-api are multicluster. - if cluster == "" { - cluster = globalPackagingCluster - } fn := clientgetter.NewHelmActionConfigGetter(configGetter, cluster) return fn(ctx, pkgContext.GetNamespace()) }, diff --git a/pkg/kube/kube_handler.go b/pkg/kube/kube_handler.go index 5a34b0e1694..60ebc3a4b3f 100644 --- a/pkg/kube/kube_handler.go +++ b/pkg/kube/kube_handler.go @@ -36,7 +36,15 @@ import ( log "k8s.io/klog/v2" ) -const OCIImageManifestMediaType = "application/vnd.oci.image.manifest.v1+json" +const ( + OCIImageManifestMediaType = "application/vnd.oci.image.manifest.v1+json" + // Kubeapps can be configured such that users cannot target the cluster + // on which Kubeapps is itself installed (ie. it's not listed in the + // clusters config). In this specific case, there is no way to refer + // to a configured name for the global packaging cluster, so we define + // one to be used that does not clash with user-configurable names. + KUBEAPPS_GLOBAL_PACKAGING_CLUSTER_TOKEN = "-" +) // ClusterConfig contains required info to talk to additional clusters. type ClusterConfig struct { @@ -106,10 +114,11 @@ func NewClusterConfig(inClusterConfig *rest.Config, userToken string, cluster st config.BearerToken = userToken config.BearerTokenFile = "" - // If the cluster is empty, we assume the rest of the inClusterConfig is correct. This can be the case when - // the cluster on which Kubeapps is installed is not one presented in the UI as a target (hence not in the - // `clusters` configuration). - if cluster == "" { + // If the cluster name is the Kubeapps global packaging cluster then the + // inClusterConfig is already correct. This can be the case when the cluster + // on which Kubeapps is installed is not one presented in the UI as a target + // (hence not in the `clusters` configuration). + if cluster == KUBEAPPS_GLOBAL_PACKAGING_CLUSTER_TOKEN { return config, nil } @@ -207,6 +216,12 @@ func ParseClusterConfig(configPath, caFilesPrefix string, pinnipedProxyURL strin } configs.Clusters[c.Name] = c } + // If the cluster on which Kubeapps is installed was not present in + // the clusters config, we explicitly use a token to identify this + // cluster when needed (such as for global available packages). + if configs.KubeappsClusterName == "" { + configs.KubeappsClusterName = KUBEAPPS_GLOBAL_PACKAGING_CLUSTER_TOKEN + } return configs, deferFn, nil } diff --git a/pkg/kube/kube_handler_test.go b/pkg/kube/kube_handler_test.go index c9a51bb3fa2..67787b33855 100644 --- a/pkg/kube/kube_handler_test.go +++ b/pkg/kube/kube_handler_test.go @@ -1634,9 +1634,9 @@ func TestNewClusterConfig(t *testing.T) { }, }, { - name: "returns an in-cluster config when no cluster is specified", + name: "returns an in-cluster config when the global packaging cluster token is specified", userToken: "token-1", - cluster: "", + cluster: KUBEAPPS_GLOBAL_PACKAGING_CLUSTER_TOKEN, clustersConfig: ClustersConfig{ KubeappsClusterName: "", Clusters: map[string]ClusterConfig{ @@ -2029,7 +2029,7 @@ func TestParseClusterConfig(t *testing.T) { {"name": "cluster-3", "apiServiceURL": "https://example.com/cluster-3", "certificateAuthorityData": "Y2EtY2VydC1kYXRhLWFkZGl0aW9uYWwK"} ]`, expectedConfig: ClustersConfig{ - KubeappsClusterName: "", + KubeappsClusterName: KUBEAPPS_GLOBAL_PACKAGING_CLUSTER_TOKEN, Clusters: map[string]ClusterConfig{ "cluster-2": { Name: "cluster-2", From b9e3d500ef7e749d43aebcd42e4eb7267a270368 Mon Sep 17 00:00:00 2001 From: Michael Nelson Date: Tue, 19 Apr 2022 13:58:27 +1000 Subject: [PATCH 2/2] Use new global packaging cluster token in test. Signed-off-by: Michael Nelson --- cmd/kubeapps-apis/core/plugins/v1alpha1/plugins_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/kubeapps-apis/core/plugins/v1alpha1/plugins_test.go b/cmd/kubeapps-apis/core/plugins/v1alpha1/plugins_test.go index f5ec3a92ba2..90d9fba9e79 100644 --- a/cmd/kubeapps-apis/core/plugins/v1alpha1/plugins_test.go +++ b/cmd/kubeapps-apis/core/plugins/v1alpha1/plugins_test.go @@ -367,6 +367,7 @@ func TestCreateConfigGetterWithParams(t *testing.T) { name: "it creates the config for the default cluster when passing a valid value for the authorization metadata", contextKey: "authorization", contextValue: "Bearer abc", + cluster: kube.KUBEAPPS_GLOBAL_PACKAGING_CLUSTER_TOKEN, expectedAPIHost: DefaultK8sAPI, expectedErrMsg: nil, },