rbd: create token and use it for vault SA #3174
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rakshith-R
force-pushed
the
madhu/service-account
branch
2 times, most recently
from
29fc8ef
to
078238d
Compare
|
/retest all |
Madhu-1
reviewed
Jun 9, 2022
| @@ -31,4 +31,7 @@ rules: | |||
| - apiGroups: ["storage.k8s.io"] | |||
| resources: ["volumeattachments"] | |||
| verbs: ["list", "get"] | |||
| - apiGroups: ["authentication.k8s.io"] | |||
There was a problem hiding this comment.
same change is required in Rook, dont forget to send patch to Rook
Madhu-1
reviewed
Jun 9, 2022
internal/kms/vault_sa.go
Outdated
| @@ -303,6 +307,27 @@ func (kms *vaultTenantSA) getToken() (string, error) { | |||
| } | |||
| } | |||
|
|
|||
| version, err := c.ServerVersion() | |||
| if err != nil { | |||
| return "", fmt.Errorf("can not get ServiceVersion %w", err) | |||
There was a problem hiding this comment.
Suggested change
| return "", fmt.Errorf("can not get ServiceVersion %w", err) | |
| return "", fmt.Errorf("failed to get ServiceVersion %w", err) |
internal/kms/vault_sa.go
Outdated
| } | ||
| // from kubernetes v1.24+, secret for service account tokens are not | ||
| // automatically created. Hence, use the create token api to fetch it. | ||
| if version.Major == kubeMajorVersion && version.Minor >= kubeMinorVersion { |
There was a problem hiding this comment.
add kubernetes changelog link here for future reference?
nixpanic
requested changes
Jun 9, 2022
internal/kms/vault_sa.go
Outdated
| @@ -35,6 +36,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 to get token from the ServiceAccount. | |||
| kubeMajorVersion = "1" | |||
There was a problem hiding this comment.
the name of these constants do not say anything about the feature that Kubernetes introduced. Please use a better name.
internal/kms/vault_sa.go
Outdated
| @@ -303,6 +307,27 @@ func (kms *vaultTenantSA) getToken() (string, error) { | |||
| } | |||
| } | |||
|
|
|||
| version, err := c.ServerVersion() | |||
There was a problem hiding this comment.
move this to a helper function, one that can always be called. If the version does not need to create a token, just return no error.
internal/kms/vault_sa.go
Outdated
| @@ -303,6 +307,27 @@ func (kms *vaultTenantSA) getToken() (string, error) { | |||
| } | |||
| } | |||
|
|
|||
| version, err := c.ServerVersion() | |||
| if err != nil { | |||
| return "", fmt.Errorf("can not get ServiceVersion %w", err) | |||
There was a problem hiding this comment.
use the standard format "<message>: %w", with :
|
https://jenkins-ceph-csi.apps.ocp.ci.centos.org/blue/organizations/jenkins/mini-e2e-helm_k8s-1.24/detail/mini-e2e-helm_k8s-1.24/18/pipeline/ @pkalever , rbd nbd tests are failing |
Rakshith-R
force-pushed
the
madhu/service-account
branch
2 times, most recently
from
90e3127
to
30a6338
Compare
|
I've tested it manually on k8s 1.24 and it works logs |
Rakshith-R
force-pushed
the
madhu/service-account
branch
from
30a6338
to
286e997
Compare
Rakshith-R
force-pushed
the
madhu/service-account
branch
from
286e997
to
0456969
Compare
Madhu-1
previously approved these changes
Jun 14, 2022
Rakshith-R
added a commit
to Rakshith-R/rook
that referenced
this pull request
Jun 14, 2022
This rbac is required to fetch serviceaccount token for vault tenant sa encryption type on k8s 1.24+. refer: ceph/ceph-csi#3174 Signed-off-by: Rakshith R <rar@redhat.com>
Rakshith-R
added a commit
to Rakshith-R/rook
that referenced
this pull request
Jun 14, 2022
This rbac is required to fetch serviceaccount token for vault tenant sa encryption type on k8s 1.24+. refer: ceph/ceph-csi#3174 Signed-off-by: Rakshith R <rar@redhat.com>
7 tasks
yati1998
previously approved these changes
Jun 14, 2022
|
/retest ci/centos/mini-e2e-helm/k8s-1.22 |
|
/retest ci/centos/mini-e2e-helm/k8s-1.23 |
|
@Mergifyio requeue |
☑️ This pull request is already queued |
|
/retest ci/centos/k8s-e2e-external-storage/1.22 |
|
@Rakshith-R "ci/centos/k8s-e2e-external-storage/1.22" test failed. Logs are available at location for debugging |
|
@Mergifyio requeue |
☑️ This pull request is already queued |
|
/retest ci/centos/mini-e2e-helm/k8s-1.21 |
|
@Rakshith-R "ci/centos/mini-e2e-helm/k8s-1.21" test failed. Logs are available at location for debugging |
|
/retest ci/centos/k8s-e2e-external-storage/1.22 |
|
@Rakshith-R "ci/centos/k8s-e2e-external-storage/1.22" test failed. Logs are available at location for debugging |
|
/retest ci/centos/k8s-e2e-external-storage/1.21 |
|
@Rakshith-R "ci/centos/k8s-e2e-external-storage/1.21" test failed. Logs are available at location for debugging |
|
@Mergifyio requeue |
☑️ This pull request is already queued |
|
/retest ci/centos/k8s-e2e-external-storage/1.22 |
|
@Rakshith-R "ci/centos/k8s-e2e-external-storage/1.22" test failed. Logs are available at location for debugging |
|
@Mergifyio requeue |
☑️ This pull request is already queued |
|
/retest ci/centos/mini-e2e-helm/k8s-1.21 |
|
@Rakshith-R "ci/centos/mini-e2e-helm/k8s-1.21" test failed. Logs are available at location for debugging |
|
@Mergifyio requeue |
☑️ This pull request is already queued |
4 tasks
mergify bot
pushed a commit
to rook/rook
that referenced
this pull request
Jun 19, 2022
This rbac is required to fetch serviceaccount token for vault tenant sa encryption type on k8s 1.24+. refer: ceph/ceph-csi#3174 Signed-off-by: Rakshith R <rar@redhat.com> (cherry picked from commit 623c515)
Rakshith-R
added a commit
to Rakshith-R/rook
that referenced
this pull request
Jun 20, 2022
This rbac is required to fetch serviceaccount token for vault tenant sa encryption type on k8s 1.24+. refer: ceph/ceph-csi#3174 Signed-off-by: Rakshith R <rar@redhat.com> (cherry picked from commit 623c515) (cherry picked from commit 54bf464)
10 tasks
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/rook
that referenced
this pull request
Jun 21, 2022
This rbac is required to fetch serviceaccount token for vault tenant sa encryption type on k8s 1.24+. refer: ceph/ceph-csi#3174 Signed-off-by: Rakshith R <rar@redhat.com> (cherry picked from commit 623c515) (cherry picked from commit 54bf464)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
create the token if kubernetes version is 1.24+ and use it for vault sa.
Signed-off-by: Madhu Rajanna madhupr007@gmail.com
Signed-off-by: Rakshith R rar@redhat.com
Resolves: #3135