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

Move CredentialProvider into its own package #363

Merged
merged 9 commits into from
Feb 3, 2025

Conversation

unexge
Copy link
Contributor

@unexge unexge commented Jan 30, 2025

Supersedes #328. Pre-work for #279.

This PR moves the logic in CredentialProvider, MountCredentials, awsprofile package and tokenFileTender function (in driver.go for driver-level IRSA) into credentialprovider package to unify all credential related logic.

Addition to that, it makes the following changes to make it workable with the upcoming PodMounter for containerization:

  • Move credentialprovider.Provide method call into Mounter interface as SystemdMounter and PodMounter will have different mechanism/paths to provide credentials
  • Make path to write credentials a parameter of credentialprovider.{Provide, Cleanup} as SystemdMounter and PodMounter will use different paths to write credentials
  • Move calling Cleanup to Unmount method of Mounter interface

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@unexge unexge requested a review from a team as a code owner January 30, 2025 16:47
@@ -0,0 +1,162 @@
// Package awsprofile provides utilities for creating and deleting AWS Profile (i.e., credentials & config files).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This package now accepts basepath, prefix and filePerm settings as arguments (see Setting struct) to accommodate changes in credentialprovider. There is no other change than that.

Comment on lines -93 to -99
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
tokenFile := os.Getenv(webIdentityTokenEnv)
if tokenFile != "" {
klog.Infof("Found AWS_WEB_IDENTITY_TOKEN_FILE, syncing token")
go tokenFileTender(ctx, tokenFile, "/csi/token")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This logic has been moved to provideStsWebIdentityCredentialsFromDriver function in pkg/driver/node/credentialprovider/provider_driver.go. Since we enabled requiresRepublish as part of Pod-level identity support, Kubernetes will call NodePublishVolume periodically to update existing service account tokens before they expire, and the credential provider will be called as part of this method. Another reason for this change is that, this assumes a single location for service account tokens for Driver-level identity, but with containerization this won't be the case. See the note regarding credential provider in the original PR description for more context.

@@ -0,0 +1,84 @@
package credentialprovider
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no change in stsRegion, RegionFromIMDSOnce and its tests rather than using the new ProvideContext struct instead of volume context.

Comment on lines -135 to -145
if credentials.AccessKeyID != "" && credentials.SecretAccessKey != "" {
// Kubernetes creates target path in the form of "/var/lib/kubelet/pods/<pod-uuid>/volumes/kubernetes.io~csi/<volume-id>/mount".
// So the directory of the target path is unique for this mount, and we can use it to write credentials and config files.
// These files will be cleaned up in `Unmount`.
basepath := filepath.Dir(target)
awsProfile, err = awsprofile.CreateAWSProfile(basepath, credentials.AccessKeyID, credentials.SecretAccessKey, credentials.SessionToken)
if err != nil {
klog.V(4).Infof("Mount: Failed to create AWS Profile in %s: %v", basepath, err)
return fmt.Errorf("Mount: Failed to create AWS Profile in %s: %v", basepath, err)
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This logic to create AWS profile based on long-term credentials has been moved to provideLongTermCredentialsFromDriver function in pkg/driver/node/credentialprovider/provider_driver.go.

Comment on lines +201 to +215
func (m *SystemdMounter) credentialWriteAndEnvPath() (writePath string, envPath string) {
// This is the plugin directory for CSI driver mounted in the container.
writePath = "/csi"
// This is the plugin directory for CSI driver in the host.
envPath = hostPluginDirWithDefault()
return writePath, envPath
}

func hostPluginDirWithDefault() string {
hostPluginDir := os.Getenv("HOST_PLUGIN_DIR")
if hostPluginDir == "" {
hostPluginDir = "/var/lib/kubelet/plugins/s3.csi.aws.com/"
}
return hostPluginDir
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This two path, writePath and envPath was hard-coded in CredentialProvider before and now an argument to credentialProvider.{Provide, Cleanup} as they will be different in PodMounter.

Comment on lines -71 to -79
volumeCtx := req.GetVolumeContext()
if volumeCtx[volumecontext.AuthenticationSource] == mounter.AuthenticationSourcePod {
podID := volumeCtx[volumecontext.CSIPodUID]
volumeID := req.GetVolumeId()
if podID != "" && volumeID != "" {
err := ns.credentialProvider.CleanupToken(volumeID, podID)
if err != nil {
klog.V(4).Infof("NodeStageVolume: Failed to cleanup token for pod/volume %s/%s: %v", podID, volumeID, err)
}
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This NodeStageVolume wasn't enabled at all – we need to add STAGE_UNSTAGE_VOLUME node capability to enable – and also this cleaning of tokens has been moved to Unmount method of mounter.

// filesystems, and to communicate with each other, the CSI Driver Node Pod uses `hostPath` volume to gain
// access some path visible from both the CSI Driver Node Pod and Mountpoint, and setups files in that volume
// using [WritePath] and returns paths to these files in [EnvPath], so Mountpoint can correctly read these files.
type ProvideContext struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

credentialprovider.Provide method was accepting a few arguments including volume context before, now I grouped all them into ProvideContext.

The WritePath and EnvPath are the new ones, they were hard-coded before, and will be different on PodMounter and SystemdMounter.

@unexge unexge force-pushed the add-credentialprovider-package branch from 49f7629 to 314f370 Compare February 3, 2025 10:29
@unexge unexge enabled auto-merge February 3, 2025 12:59
@unexge unexge requested a review from muddyfish February 3, 2025 15:16
@unexge unexge added this pull request to the merge queue Feb 3, 2025
Merged via the queue into awslabs:main with commit 1c5acb1 Feb 3, 2025
27 checks passed
@unexge unexge deleted the add-credentialprovider-package branch February 3, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants