Skip to content

[release-4.19] OCPBUGS-63137: client: re-use a single file for building the session instead of randomly named files #152

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 24, 2025
Merged

[release-4.19] OCPBUGS-63137: client: re-use a single file for building the session instead of randomly named files #152

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 35 additions & 12 deletions pkg/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import (
"bytes"
"context"
"fmt"
"io/ioutil"
"os"
"path"
"strings"
"sync"
"time"
Expand All @@ -45,6 +45,7 @@ import (
configv1 "github.com/openshift/api/config/v1"
machineapiapierrors "github.com/openshift/machine-api-operator/pkg/controller/machine"
apimachineryerrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/util/rand"
)

//go:generate go run ../../vendor/github.com/golang/mock/mockgen -source=./client.go -destination=./mock/client_generated.go -package=mock
Expand All @@ -68,6 +69,11 @@ const (
awsRegionsCacheExpirationDuration = time.Minute * 30
)

var (
sharedCredentialsFileMutex sync.Mutex
sharedCredentialsFileName = path.Join(os.TempDir(), "aws-shared-credentials"+rand.String(16))
)

// AwsClientBuilderFuncType is function type for building aws client
type AwsClientBuilderFuncType func(client client.Client, secretName, namespace, region string, configManagedClient client.Client, regionCache RegionCache) (Client, error)

Expand Down Expand Up @@ -354,7 +360,7 @@ func NewValidatedClient(ctrlRuntimeClient client.Client, secretName, namespace,
}, nil
}

func newAWSSession(ctrlRuntimeClient client.Client, secretName, namespace, region string, configManagedClient client.Client) (*session.Session, error) {
func newAWSSession(ctrlRuntimeClient client.Client, secretName, namespace, region string, configManagedClient client.Client) (s *session.Session, err error) {
sessionOptions := session.Options{
Config: aws.Config{
Region: aws.String(region),
Expand All @@ -369,10 +375,20 @@ func newAWSSession(ctrlRuntimeClient client.Client, secretName, namespace, regio
}
return nil, err
}
sharedCredentialsFileMutex.Lock()
defer sharedCredentialsFileMutex.Unlock()
sharedCredsFile, err := sharedCredentialsFileFromSecret(&secret)
if err != nil {
return nil, fmt.Errorf("failed to create shared credentials file from Secret: %v", err)
}

// Ensure the file gets deleted in any case.
defer func() {
if removeErr := os.Remove(sharedCredsFile); removeErr != nil && err == nil {
err = fmt.Errorf("failed to remove shared credentials file %s: %v", sharedCredsFile, removeErr)
}
}()

sessionOptions.SharedConfigState = session.SharedConfigEnable
sessionOptions.SharedConfigFiles = []string{sharedCredsFile}
}
Expand All @@ -387,16 +403,11 @@ func newAWSSession(ctrlRuntimeClient client.Client, secretName, namespace, regio
}

// Otherwise default to relying on the IAM role of the masters where the actuator is running:
s, err := session.NewSessionWithOptions(sessionOptions)
s, err = session.NewSessionWithOptions(sessionOptions)
if err != nil {
return nil, err
}

// Remove any temporary shared credentials files after session creation so they don't accumulate
if len(sessionOptions.SharedConfigFiles) > 0 {
os.Remove(sessionOptions.SharedConfigFiles[0])
}

s.Handlers.Build.PushBackNamed(addProviderVersionToUserAgent)

return s, nil
Expand Down Expand Up @@ -457,7 +468,7 @@ func buildCustomEndpointsMap(customEndpoints []configv1.AWSServiceEndpoint) map[

// sharedCredentialsFileFromSecret returns a location (path) to the shared credentials
// file that was created using the provided secret
func sharedCredentialsFileFromSecret(secret *corev1.Secret) (string, error) {
func sharedCredentialsFileFromSecret(secret *corev1.Secret) (filename string, err error) {
var data []byte
switch {
case len(secret.Data["credentials"]) > 0:
Expand All @@ -471,12 +482,24 @@ func sharedCredentialsFileFromSecret(secret *corev1.Secret) (string, error) {
return "", fmt.Errorf("invalid secret for aws credentials")
}

f, err := ioutil.TempFile("", "aws-shared-credentials")
// Re-using the same file every time to prevent leakage of memory to slab.
// Related issue: https://issues.redhat.com/browse/RHEL-119532
f, err := os.Create(sharedCredentialsFileName)
if err != nil {
return "", fmt.Errorf("failed to create file for shared credentials: %v", err)
}
defer f.Close()
if _, err := f.Write(data); err != nil {

defer func() {
if closeErr := f.Close(); closeErr != nil && err == nil {
err = fmt.Errorf("failed to close file %s: %v", f.Name(), closeErr)
}
}()

if _, err = f.Write(data); err != nil {
// Delete the file in case of having an error. Otherwise the calling function needs to handle deletion.
if deleteErr := os.Remove(f.Name()); deleteErr != nil {
return "", fmt.Errorf("failed to write credentials to %s and delete it afterwards: %v, %v", f.Name(), err, deleteErr)
}
return "", fmt.Errorf("failed to write credentials to %s: %v", f.Name(), err)
}
return f.Name(), nil
Expand Down