Skip to content

Commit

Permalink
Configuration of service account token for additional clusters (#5286)
Browse files Browse the repository at this point in the history 
* Added logic from #5034

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

* Move the SA token to its own client getter

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

* Added missing error check

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

* Fix for return value

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

* Setting additional SA token dynamically

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

* Fix wrong command

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>

Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
  • Loading branch information
@castelblanque
castelblanque authored Sep 7, 2022
1 parent 04c26fa commit 55bfe30
Show file tree
Hide file tree
Showing 9 changed files with 184 additions and 42 deletions.
2 changes: 1 addition & 1 deletion cmd/kubeapps-apis/plugins/resources/v1alpha1/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func init() {
//
//nolint:deadcode
func RegisterWithGRPCServer(opts pluginsv1alpha1.GRPCPluginRegistrationOptions) (interface{}, error) {
svr, err := NewServer(opts.ConfigGetter, opts.ClientQPS, opts.ClientBurst, opts.PluginConfigPath)
svr, err := NewServer(opts.ConfigGetter, opts.ClientQPS, opts.ClientBurst, opts.PluginConfigPath, opts.ClustersConfig)
if err != nil {
return nil, err
}
Expand Down
11 changes: 10 additions & 1 deletion cmd/kubeapps-apis/plugins/resources/v1alpha1/namespaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package main
import (
"context"
authorizationapi "k8s.io/api/authorization/v1"
"k8s.io/client-go/kubernetes"
"strings"

"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/plugins/resources/v1alpha1"
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/statuserror"
Expand Down Expand Up @@ -112,7 +114,14 @@ func (s *Server) CanI(ctx context.Context, r *v1alpha1.CanIRequest) (*v1alpha1.C
}
log.InfoS("+resources CanI", "cluster", cluster, "namespace", namespace, "group", r.GetGroup(), "resource", r.GetResource(), "verb", r.GetVerb())

typedClient, _, err := s.clientGetter(ctx, cluster)
var typedClient kubernetes.Interface
var err error
if s.kubeappsCluster != cluster && strings.ToLower(r.GetVerb()) == "list" && strings.ToLower(r.GetResource()) == "namespaces" {
// Listing namespaces in additional clusters might involve using the provided service account token
typedClient, _, err = s.clusterServiceAccountClientGetter(ctx, cluster)
} else {
typedClient, _, err = s.clientGetter(ctx, cluster)
}
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to get the k8s client: '%v'", err)
}
Expand Down
10 changes: 5 additions & 5 deletions cmd/kubeapps-apis/plugins/resources/v1alpha1/namespaces_filtering.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,22 @@ func (s *Server) GetAccessibleNamespaces(ctx context.Context, cluster string, tr
namespaceList = append(namespaceList, trustedNamespaces...)
} else {

typedClient, _, err := s.clientGetter(ctx, cluster)
typedClient, _, err := s.clusterServiceAccountClientGetter(ctx, cluster)
if err != nil {
return nil, status.Errorf(codes.Internal, "unable to get the k8s client: '%v'", err)
}

// Try to list namespaces with the user token, for backward compatibility
namespaces, err := typedClient.CoreV1().Namespaces().List(context.TODO(), metav1.ListOptions{})
backgroundCtx := context.Background()
namespaces, err := typedClient.CoreV1().Namespaces().List(backgroundCtx, metav1.ListOptions{})
if err != nil {
if k8sErrors.IsForbidden(err) {
backgroundCtx := context.Background()
// The user doesn't have permissions to list namespaces, use the current pod's service account
userClient, err := s.serviceAccountClientGetter.Typed(backgroundCtx)
userClient, err := s.localServiceAccountClientGetter.Typed(backgroundCtx)
if err != nil {
return nil, err
}
namespaces, err = userClient.CoreV1().Namespaces().List(context.TODO(), metav1.ListOptions{})
namespaces, err = userClient.CoreV1().Namespaces().List(backgroundCtx, metav1.ListOptions{})
if err != nil && k8sErrors.IsForbidden(err) {
// Not even the configured kubeapps-apis service account has permission
return nil, err
Expand Down
13 changes: 8 additions & 5 deletions cmd/kubeapps-apis/plugins/resources/v1alpha1/namespaces_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,9 +611,12 @@ func TestGetNamespaceNames(t *testing.T) {
clientGetter: func(context.Context, string) (kubernetes.Interface, dynamic.Interface, error) {
return fakeClient, nil, nil
},
serviceAccountClientGetter: backgroundClientGetter,
clientQPS: 5,
pluginConfig: pluginConfig,
clusterServiceAccountClientGetter: func(context.Context, string) (kubernetes.Interface, dynamic.Interface, error) {
return fakeClient, nil, nil
},
localServiceAccountClientGetter: backgroundClientGetter,
clientQPS: 5,
pluginConfig: pluginConfig,
}

ctx := context.Background()
Expand Down Expand Up @@ -714,8 +717,8 @@ func TestCanI(t *testing.T) {
clientGetter: func(context.Context, string) (kubernetes.Interface, dynamic.Interface, error) {
return fakeClient, nil, nil
},
serviceAccountClientGetter: backgroundClientGetter,
clientQPS: 5,
localServiceAccountClientGetter: backgroundClientGetter,
clientQPS: 5,
}

response, err := s.CanI(context.Background(), tc.request)
Expand Down
82 changes: 56 additions & 26 deletions cmd/kubeapps-apis/plugins/resources/v1alpha1/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/clientgetter"
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/resources/v1alpha1/common"
"github.com/vmware-tanzu/kubeapps/pkg/kube"
"os"
"sync"

Expand Down Expand Up @@ -46,9 +47,12 @@ type Server struct {
// non-test implementation.
clientGetter clientGetter

// clusterServiceAccountClientGetter gets a client getter with service account for additional clusters
clusterServiceAccountClientGetter clientGetter

// for interactions with k8s API server in the context of
// kubeapps-internal-kubeappsapis service account
serviceAccountClientGetter clientgetter.BackgroundClientGetterFunc
localServiceAccountClientGetter clientgetter.BackgroundClientGetterFunc

// corePackagesClientGetter holds a function to obtain the core.packages.v1alpha1
// client. It is similarly initialised in NewServer() below.
Expand All @@ -67,6 +71,8 @@ type Server struct {
pluginConfig *common.ResourcesPluginConfig

clientQPS float32

kubeappsCluster string
}

// createRESTMapper returns a rest mapper configured with the APIs of the
Expand Down Expand Up @@ -101,7 +107,7 @@ func createRESTMapper(clientQPS float32, clientBurst int) (meta.RESTMapper, erro
return restmapper.NewDiscoveryRESTMapper(groupResources), nil
}

func NewServer(configGetter core.KubernetesConfigGetter, clientQPS float32, clientBurst int, pluginConfigPath string) (*Server, error) {
func NewServer(configGetter core.KubernetesConfigGetter, clientQPS float32, clientBurst int, pluginConfigPath string, clustersConfig kube.ClustersConfig) (*Server, error) {
mapper, err := createRESTMapper(clientQPS, clientBurst)
if err != nil {
return nil, err
Expand All @@ -119,29 +125,13 @@ func NewServer(configGetter core.KubernetesConfigGetter, clientQPS float32, clie
log.Info("+resources using default config since pluginConfigPath is empty")
}

// Get the "in-cluster" client getter
backgroundClientGetter := clientgetter.NewBackgroundClientGetter(configGetter, clientgetter.Options{})

return &Server{
clientGetter: func(ctx context.Context, cluster string) (kubernetes.Interface, dynamic.Interface, error) {
if configGetter == nil {
return nil, nil, status.Errorf(codes.Internal, "configGetter arg required")
}
config, err := configGetter(ctx, cluster)
if err != nil {
return nil, nil, status.Errorf(codes.FailedPrecondition, "unable to get config : %v", err.Error())
}
dynamicClient, err := dynamic.NewForConfig(config)
if err != nil {
return nil, nil, status.Errorf(codes.FailedPrecondition, "unable to get dynamic client : %s", err.Error())
}
typedClient, err := kubernetes.NewForConfig(config)
if err != nil {
return nil, nil, status.Errorf(codes.FailedPrecondition, "unable to get typed client: %s", err.Error())
}
return typedClient, dynamicClient, nil
},
serviceAccountClientGetter: backgroundClientGetter,
// Get the client getter with context auth
clientGetter: newClientGetter(configGetter, false, clustersConfig),
// Get the additional cluster client getter with service account
clusterServiceAccountClientGetter: newClientGetter(configGetter, true, clustersConfig),
// Get the "in-cluster" client getter
localServiceAccountClientGetter: clientgetter.NewBackgroundClientGetter(configGetter, clientgetter.Options{}),
corePackagesClientGetter: func() (pkgsGRPCv1alpha1.PackagesServiceClient, error) {
port := os.Getenv("PORT")
conn, err := grpc.Dial("localhost:"+port, grpc.WithTransportCredentials(insecure.NewCredentials()))
Expand All @@ -158,11 +148,51 @@ func NewServer(configGetter core.KubernetesConfigGetter, clientQPS float32, clie
}
return mapping.Resource, mapping.Scope.Name(), nil
},
clientQPS: clientQPS,
pluginConfig: pluginConfig,
clientQPS: clientQPS,
pluginConfig: pluginConfig,
kubeappsCluster: clustersConfig.KubeappsClusterName,
}, nil
}

func newClientGetter(configGetter core.KubernetesConfigGetter, useServiceAccount bool, clustersConfig kube.ClustersConfig) clientGetter {
return func(ctx context.Context, cluster string) (kubernetes.Interface, dynamic.Interface, error) {
if configGetter == nil {
return nil, nil, status.Errorf(codes.Internal, "configGetter arg required")
}
restConfig, err := configGetter(ctx, cluster)
if err != nil {
return nil, nil, status.Errorf(codes.FailedPrecondition, "unable to get config : %v", err.Error())
}
if err := setupRestConfigForCluster(restConfig, cluster, useServiceAccount, clustersConfig); err != nil {
return nil, nil, err
}
dynamicClient, err := dynamic.NewForConfig(restConfig)
if err != nil {
return nil, nil, status.Errorf(codes.FailedPrecondition, "unable to get dynamic client : %s", err.Error())
}
typedClient, err := kubernetes.NewForConfig(restConfig)
if err != nil {
return nil, nil, status.Errorf(codes.FailedPrecondition, "unable to get typed client: %s", err.Error())
}
return typedClient, dynamicClient, nil
}
}

func setupRestConfigForCluster(restConfig *rest.Config, cluster string, useServiceAccount bool, clustersConfig kube.ClustersConfig) error {
// Override client config with the service token for additional cluster
// Added from #5034 after deprecation of "kubeops"
if cluster != clustersConfig.KubeappsClusterName && useServiceAccount {
additionalCluster, ok := clustersConfig.Clusters[cluster]
if !ok {
return status.Errorf(codes.Internal, "cluster %q has no configuration", cluster)
}
if additionalCluster.ServiceToken != "" {
restConfig.BearerToken = additionalCluster.ServiceToken
}
}
return nil
}

// GetResources returns the resources for an installed package.
func (s *Server) GetResources(r *v1alpha1.GetResourcesRequest, stream v1alpha1.ResourcesService_GetResourcesServer) error {
namespace := r.GetInstalledPackageRef().GetContext().GetNamespace()
Expand Down
101 changes: 101 additions & 0 deletions cmd/kubeapps-apis/plugins/resources/v1alpha1/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ package main
import (
"context"
"errors"
"github.com/vmware-tanzu/kubeapps/pkg/kube"
"io"
"k8s.io/client-go/rest"
"net"
"reflect"
"testing"
Expand Down Expand Up @@ -529,3 +531,102 @@ func TestGetServiceAccountNames(t *testing.T) {
})
}
}

func TestSetupConfigForCluster(t *testing.T) {
testCases := []struct {
name string
restConfig *rest.Config
cluster string
useServiceAccount bool
clustersConfig kube.ClustersConfig
expectedRestConfig *rest.Config
expectedErrorCode codes.Code
}{
{
name: "config is not modified for kubeapps cluster",
cluster: "default",
clustersConfig: kube.ClustersConfig{
KubeappsClusterName: "default",
Clusters: map[string]kube.ClusterConfig{
"default": {},
},
},
restConfig: &rest.Config{},
expectedRestConfig: &rest.Config{},
},
{
name: "config is not modified for additional clusters and no service account",
cluster: "additional-1",
clustersConfig: kube.ClustersConfig{
KubeappsClusterName: "default",
Clusters: map[string]kube.ClusterConfig{
"default": {},
"additional-1": {},
},
},
restConfig: &rest.Config{},
expectedRestConfig: &rest.Config{},
},
{
name: "config setup fails for additional clusters with no cluster config data",
cluster: "additional-1",
useServiceAccount: true,
clustersConfig: kube.ClustersConfig{
KubeappsClusterName: "default",
Clusters: map[string]kube.ClusterConfig{
"default": {},
},
},
restConfig: &rest.Config{},
expectedRestConfig: &rest.Config{},
expectedErrorCode: codes.Internal,
},
{
name: "config is not modified for additional clusters with no configured service token",
cluster: "additional-1",
useServiceAccount: true,
clustersConfig: kube.ClustersConfig{
KubeappsClusterName: "default",
Clusters: map[string]kube.ClusterConfig{
"default": {},
"additional-1": {},
},
},
restConfig: &rest.Config{},
expectedRestConfig: &rest.Config{},
},
{
name: "config is modified for additional clusters when configured service token",
cluster: "additional-1",
useServiceAccount: true,
clustersConfig: kube.ClustersConfig{
KubeappsClusterName: "default",
Clusters: map[string]kube.ClusterConfig{
"default": {},
"additional-1": {
ServiceToken: "service-token-1",
},
},
},
restConfig: &rest.Config{},
expectedRestConfig: &rest.Config{
BearerToken: "service-token-1",
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {

err := setupRestConfigForCluster(tc.restConfig, tc.cluster, tc.useServiceAccount, tc.clustersConfig)

if got, want := status.Code(err), tc.expectedErrorCode; !cmp.Equal(got, want, nil) {
t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, nil))
}

if got, want := tc.restConfig, tc.expectedRestConfig; !cmp.Equal(got, want, nil) {
t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, nil))
}
})
}
}
2 changes: 1 addition & 1 deletion script/e2e-test.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ if [ "$USE_MULTICLUSTER_OIDC_ENV" = true ]; then
"--set" "clusters[1].name=second-cluster"
"--set" "clusters[1].apiServiceURL=https://${ADDITIONAL_CLUSTER_IP}:6443"
"--set" "clusters[1].insecure=true"
"--set" "clusters[1].serviceToken=ZXlKaGJHY2lPaUpTVXpJMU5pSXNJbXRwWkNJNklsbHpiSEp5TlZwM1QwaG9WSE5PYkhVdE5GQkRablY2TW0wd05rUmtMVmxFWVV4MlZEazNaeTEyUmxFaWZRLmV5SnBjM01pT2lKcmRXSmxjbTVsZEdWekwzTmxjblpwWTJWaFkyTnZkVzUwSWl3aWEzVmlaWEp1WlhSbGN5NXBieTl6WlhKMmFXTmxZV05qYjNWdWRDOXVZVzFsYzNCaFkyVWlPaUprWldaaGRXeDBJaXdpYTNWaVpYSnVaWFJsY3k1cGJ5OXpaWEoyYVdObFlXTmpiM1Z1ZEM5elpXTnlaWFF1Ym1GdFpTSTZJbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmt0ZEc5clpXNHRjV295Ym1naUxDSnJkV0psY201bGRHVnpMbWx2TDNObGNuWnBZMlZoWTJOdmRXNTBMM05sY25acFkyVXRZV05qYjNWdWRDNXVZVzFsSWpvaWEzVmlaV0Z3Y0hNdGJtRnRaWE53WVdObExXUnBjMk52ZG1WeWVTSXNJbXQxWW1WeWJtVjBaWE11YVc4dmMyVnlkbWxqWldGalkyOTFiblF2YzJWeWRtbGpaUzFoWTJOdmRXNTBMblZwWkNJNkltVXhaakE1WmpSakxUTTRNemt0TkRJME15MWhZbUptTFRKaU5HWm1OREZrWW1RMllTSXNJbk4xWWlJNkluTjVjM1JsYlRwelpYSjJhV05sWVdOamIzVnVkRHBrWldaaGRXeDBPbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmtpZlEuTnh6V2dsUGlrVWpROVQ1NkpWM2xJN1VWTUVSR3J2bklPSHJENkh4dUVwR0luLWFUUzV5Q0pDa3Z0cTF6S3Z3b05sc2MyX0YxaTdFOUxWRGFwbC1UQlhleUN5Rl92S1B1TDF4dTdqZFBMZ1dKT1pQX3JMcXppaDV4ZlkxalFoOHNhdTRZclFJLUtqb3U1UkRRZ0tOQS1BaS1lRlFOZVh2bmlUNlBKYWVkc184V0t3dHRMMC1wdHpYRnBnOFl5dkx6N0U1UWdTR2tjNWpDVXlsS0RvZVRUaVRSOEc2RHFHYkFQQUYwREt0b3MybU9Geno4SlJYNHhoQmdvaUcxVTVmR1g4Z3hnTU1SV0VHRE9kaGMyeXRvcFdRUkRpYmhvaldNS3VDZlNua09zMDRGYTBkYmEwQ0NTbld2a29LZ3Z4QVR5aVVrWm9wV3VpZ1JJNFd5dDkzbXhR"
"--set" "clusters[1].serviceToken=$(kubectl --context=kind-kubeapps-ci-additional --kubeconfig=${HOME}/.kube/kind-config-kubeapps-ci-additional get secret kubeapps-namespace-discovery -o go-template='{{.data.token | base64decode}}')"
)
fi

Expand Down
3 changes: 2 additions & 1 deletion script/makefiles/deploy-dev.mk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ deploy-dev-kubeapps:
helm --kubeconfig=${CLUSTER_CONFIG} upgrade --install kubeapps ./chart/kubeapps --namespace kubeapps --create-namespace \
--values ./site/content/docs/latest/reference/manifests/kubeapps-local-dev-values.yaml \
--values ./site/content/docs/latest/reference/manifests/kubeapps-local-dev-auth-proxy-values.yaml \
--values ./site/content/docs/latest/reference/manifests/kubeapps-local-dev-additional-kind-cluster.yaml
--values ./site/content/docs/latest/reference/manifests/kubeapps-local-dev-additional-kind-cluster.yaml \
--set clusters[1].serviceToken=$(shell kubectl --kubeconfig=${ADDITIONAL_CLUSTER_CONFIG} get secret kubeapps-namespace-discovery -o go-template="{{.data.token | base64decode}}")

deploy-dev: deploy-dependencies deploy-dev-kubeapps
@echo "\nYou can now simply open your browser at https://localhost/ to access Kubeapps!"
Expand Down
2 changes: 0 additions & 2 deletions .../content/docs/latest/reference/manifests/kubeapps-local-dev-namespace-discovery-rbac.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,3 @@ metadata:
annotations:
"kubernetes.io/service-account.name": kubeapps-namespace-discovery
type: "kubernetes.io/service-account-token"
data:
token: ZXlKaGJHY2lPaUpTVXpJMU5pSXNJbXRwWkNJNklsbHpiSEp5TlZwM1QwaG9WSE5PYkhVdE5GQkRablY2TW0wd05rUmtMVmxFWVV4MlZEazNaeTEyUmxFaWZRLmV5SnBjM01pT2lKcmRXSmxjbTVsZEdWekwzTmxjblpwWTJWaFkyTnZkVzUwSWl3aWEzVmlaWEp1WlhSbGN5NXBieTl6WlhKMmFXTmxZV05qYjNWdWRDOXVZVzFsYzNCaFkyVWlPaUprWldaaGRXeDBJaXdpYTNWaVpYSnVaWFJsY3k1cGJ5OXpaWEoyYVdObFlXTmpiM1Z1ZEM5elpXTnlaWFF1Ym1GdFpTSTZJbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmt0ZEc5clpXNHRjV295Ym1naUxDSnJkV0psY201bGRHVnpMbWx2TDNObGNuWnBZMlZoWTJOdmRXNTBMM05sY25acFkyVXRZV05qYjNWdWRDNXVZVzFsSWpvaWEzVmlaV0Z3Y0hNdGJtRnRaWE53WVdObExXUnBjMk52ZG1WeWVTSXNJbXQxWW1WeWJtVjBaWE11YVc4dmMyVnlkbWxqWldGalkyOTFiblF2YzJWeWRtbGpaUzFoWTJOdmRXNTBMblZwWkNJNkltVXhaakE1WmpSakxUTTRNemt0TkRJME15MWhZbUptTFRKaU5HWm1OREZrWW1RMllTSXNJbk4xWWlJNkluTjVjM1JsYlRwelpYSjJhV05sWVdOamIzVnVkRHBrWldaaGRXeDBPbXQxWW1WaGNIQnpMVzVoYldWemNHRmpaUzFrYVhOamIzWmxjbmtpZlEuTnh6V2dsUGlrVWpROVQ1NkpWM2xJN1VWTUVSR3J2bklPSHJENkh4dUVwR0luLWFUUzV5Q0pDa3Z0cTF6S3Z3b05sc2MyX0YxaTdFOUxWRGFwbC1UQlhleUN5Rl92S1B1TDF4dTdqZFBMZ1dKT1pQX3JMcXppaDV4ZlkxalFoOHNhdTRZclFJLUtqb3U1UkRRZ0tOQS1BaS1lRlFOZVh2bmlUNlBKYWVkc184V0t3dHRMMC1wdHpYRnBnOFl5dkx6N0U1UWdTR2tjNWpDVXlsS0RvZVRUaVRSOEc2RHFHYkFQQUYwREt0b3MybU9Geno4SlJYNHhoQmdvaUcxVTVmR1g4Z3hnTU1SV0VHRE9kaGMyeXRvcFdRUkRpYmhvaldNS3VDZlNua09zMDRGYTBkYmEwQ0NTbld2a29LZ3Z4QVR5aVVrWm9wV3VpZ1JJNFd5dDkzbXhR

0 comments on commit 55bfe30

Please sign in to comment.