Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use an explicit token to identify kubeapps cluster when required. #4591

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions cmd/kubeapps-apis/core/plugins/v1alpha1/plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,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)
Expand Down
1 change: 1 addition & 0 deletions cmd/kubeapps-apis/core/plugins/v1alpha1/plugins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,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,
},
Expand Down
5 changes: 0 additions & 5 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,6 @@ func NewServer(configGetter core.KubernetesConfigGetter, globalPackagingCluster
clientGetter: clientProvider,
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())
},
Expand Down
25 changes: 20 additions & 5 deletions pkg/kube/kube_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,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 {
Expand Down Expand Up @@ -103,10 +111,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
}

Expand Down Expand Up @@ -216,6 +225,12 @@ func ParseClusterConfig(configPath, caFilesPrefix string, pinnipedProxyURL, Pinn
}
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
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/kube/kube_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1449,9 +1449,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{
Expand Down Expand Up @@ -1743,7 +1743,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",
Expand Down