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

rbd: create token and use it for vault SA #3174

Merged
merged 2 commits into from
Jun 17, 2022
Merged
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
2 changes: 1 addition & 1 deletion build.env
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ SNAPSHOT_VERSION=v6.0.1
HELM_VERSION=v3.9.0

# minikube settings
MINIKUBE_VERSION=v1.25.2
MINIKUBE_VERSION=v1.26.0-beta.1
VM_DRIVER=none
CHANGE_MINIKUBE_NONE_USER=true

Expand Down
3 changes: 3 additions & 0 deletions charts/ceph-csi-rbd/templates/nodeplugin-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,7 @@ rules:
- apiGroups: ["storage.k8s.io"]
resources: ["volumeattachments"]
verbs: ["list", "get"]
- apiGroups: [""]
resources: ["serviceaccounts/token"]
verbs: ["create"]
{{- end -}}
3 changes: 3 additions & 0 deletions charts/ceph-csi-rbd/templates/provisioner-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,8 @@ rules:
resources: ["csinodes"]
verbs: ["get", "list", "watch"]
{{- end }}
- apiGroups: [""]
resources: ["serviceaccounts/token"]
verbs: ["create"]

{{- end -}}
3 changes: 3 additions & 0 deletions deploy/rbd/kubernetes/csi-nodeplugin-rbac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ rules:
- apiGroups: ["storage.k8s.io"]
resources: ["volumeattachments"]
verbs: ["list", "get"]
- apiGroups: [""]
resources: ["serviceaccounts/token"]
verbs: ["create"]
---
kind: ClusterRoleBinding
apiVersion: rbac.authorization.k8s.io/v1
Expand Down
3 changes: 3 additions & 0 deletions deploy/rbd/kubernetes/csi-provisioner-rbac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ rules:
- apiGroups: [""]
resources: ["serviceaccounts"]
verbs: ["get"]
- apiGroups: [""]
resources: ["serviceaccounts/token"]
verbs: ["create"]
---
kind: ClusterRoleBinding
apiVersion: rbac.authorization.k8s.io/v1
Expand Down
45 changes: 41 additions & 4 deletions internal/kms/vault_sa.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,13 @@ import (
"io/ioutil"
"os"

"github.com/ceph/ceph-csi/internal/util/k8s"

"github.com/libopenstorage/secrets/vault"
authenticationv1 "k8s.io/api/authentication/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
)

const (
Expand All @@ -35,6 +39,9 @@ const (
// should be available in the Tenants namespace. This ServiceAccount
// will be used to connect to Hashicorp Vault.
vaultTenantSAName = "ceph-csi-vault-sa"
// Kubernetes version which requires ServiceAccount token creation.
// Kubernetes 1.24 => 1 * 1000 + 24.
kubeMinVersionForCreateToken = 1024
)

/*
Expand Down Expand Up @@ -292,9 +299,9 @@ func (kms *vaultTenantSA) getToken() (string, error) {
}

for _, secretRef := range sa.Secrets {
secret, err := c.CoreV1().Secrets(kms.Tenant).Get(context.TODO(), secretRef.Name, metav1.GetOptions{})
if err != nil {
return "", fmt.Errorf("failed to get Secret %s/%s: %w", kms.Tenant, secretRef.Name, err)
secret, sErr := c.CoreV1().Secrets(kms.Tenant).Get(context.TODO(), secretRef.Name, metav1.GetOptions{})
if sErr != nil {
return "", fmt.Errorf("failed to get Secret %s/%s: %w", kms.Tenant, secretRef.Name, sErr)
}

token, ok := secret.Data["token"]
Expand All @@ -303,7 +310,7 @@ func (kms *vaultTenantSA) getToken() (string, error) {
}
}

return "", fmt.Errorf("failed to find token in ServiceAccount %s/%s", kms.Tenant, kms.tenantSAName)
return kms.createToken(sa, c)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scraping the auto-generated token for use outside a pod was never correct... is there a reason not to prefer the TokenRequest approach instead of falling back to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scraping the auto-generated token for use outside a pod was never correct... is there a reason not to prefer the TokenRequest approach instead of falling back to it?

The current approach so far used to fetch the token from the secret attached and createtoken was included to support k8s 1.24's way of token request api.

We can probably only stick to token request api from following releases after making sure it stable with cephcsi?
(this patch will also be backported to 3.6)

cc @nixpanic

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I prefer to do the new way by default, and fall-back on the old method. It makes it easier to remove the legacy method at a later time.

}

// getTokenPath creates a temporary directory structure that contains the token
Expand All @@ -327,3 +334,33 @@ func (kms *vaultTenantSA) getTokenPath() (string, error) {

return dir + "/token", nil
}

// createToken creates required service account token for kubernetes 1.24+,
// else returns error.
// From kubernetes v1.24+, secret for service account tokens are not
// automatically created. Hence, use the create token api to fetch it.
// refer: https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.24.md \
// #no-really-you-must-read-this-before-you-upgrade-1 .
func (kms *vaultTenantSA) createToken(sa *corev1.ServiceAccount, client *kubernetes.Clientset) (string, error) {
major, minor, err := k8s.GetServerVersion(client)
if err != nil {
return "", fmt.Errorf("failed to get server version: %w", err)
}

if (major*1000 + minor) >= kubeMinVersionForCreateToken {
tokenRequest := &authenticationv1.TokenRequest{}
token, err := client.CoreV1().ServiceAccounts(kms.Tenant).CreateToken(
context.TODO(),
sa.Name,
tokenRequest,
metav1.CreateOptions{},
)
if err != nil {
return "", fmt.Errorf("failed to create token for service account %s/%s: %w", kms.Tenant, sa.Name, err)
}

return token.Status.Token, nil
}

return "", fmt.Errorf("failed to find token in ServiceAccount %s/%s", kms.Tenant, kms.tenantSAName)
}
45 changes: 45 additions & 0 deletions internal/util/k8s/version.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
Copyright 2022 The Ceph-CSI Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package k8s

import (
"fmt"
"strconv"

"k8s.io/client-go/kubernetes"
)

// GetServerVersion returns kubernetes server major and minor version as
// integer.
func GetServerVersion(client *kubernetes.Clientset) (int, int, error) {
version, err := client.ServerVersion()
if err != nil {
return 0, 0, fmt.Errorf("failed to get ServerVersion: %w", err)
}

major, err := strconv.Atoi(version.Major)
if err != nil {
return 0, 0, fmt.Errorf("failed to convert Kubernetes major version %q to int: %w", version.Major, err)
}

minor, err := strconv.Atoi(version.Minor)
if err != nil {
return 0, 0, fmt.Errorf("failed to convert Kubernetes minor version %q to int: %w", version.Minor, err)
}

return major, minor, nil
}