From e8118a818156f5aa87386d2d59cefbe42123ee3e Mon Sep 17 00:00:00 2001 From: Bryan Cox Date: Fri, 18 Oct 2024 15:07:19 -0400 Subject: [PATCH] Use Client Cert Auth for ARO HCP deployments Use Client Certificate Authentication for ARO HCP deployments. HyperShift will pass the needed environment variables for this authentication method: ARO_HCP_MI_CLIENT_ID, ARO_HCP_TENANT_ID, and ARO_HCP_CLIENT_CERTIFICATE_PATH. Signed-off-by: Bryan Cox --- go.mod | 2 +- pkg/dns/azure/client/auth.go | 44 ++++++++++++++++--- pkg/util/filewatcher/filewatcher.go | 65 +++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 pkg/util/filewatcher/filewatcher.go diff --git a/go.mod b/go.mod index 46ffcc46a9..31c932738c 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( github.com/aws/aws-sdk-go v1.38.49 github.com/davecgh/go-spew v1.1.1 github.com/florianl/go-nfqueue v1.3.2 + github.com/fsnotify/fsnotify v1.7.0 github.com/go-logr/logr v1.4.1 github.com/go-logr/zapr v1.3.0 github.com/google/go-cmp v0.6.0 @@ -71,7 +72,6 @@ require ( github.com/evanphx/json-patch v5.6.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.9.0 // indirect github.com/fatih/color v1.12.0 // indirect - github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/go-openapi/errors v0.19.8 // indirect github.com/go-openapi/jsonpointer v0.19.6 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect diff --git a/pkg/dns/azure/client/auth.go b/pkg/dns/azure/client/auth.go index 4a61f0c1c0..700644a57b 100644 --- a/pkg/dns/azure/client/auth.go +++ b/pkg/dns/azure/client/auth.go @@ -2,8 +2,10 @@ package client import ( "errors" + "fmt" "os" "strings" + "sync" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud" @@ -11,8 +13,12 @@ import ( "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/azure" "github.com/jongio/azidext/go/azidext" + + "github.com/openshift/cluster-ingress-operator/pkg/util/filewatcher" ) +var watchCertificateFileOnce sync.Once + func getAuthorizerForResource(config Config) (autorest.Authorizer, error) { var cloudConfig cloud.Configuration switch config.Environment { @@ -70,17 +76,43 @@ func getAuthorizerForResource(config Config) (autorest.Authorizer, error) { } var cred azcore.TokenCredential - // MSI Override for ARO HCP - msi := os.Getenv("AZURE_MSI_AUTHENTICATION") - if msi == "true" { - options := azidentity.ManagedIdentityCredentialOptions{ + // Managed Identity Override for ARO HCP. In ARO HCP, we ignore the values provided for AZURE_TENANT_ID and + // AZURE_CLIENT_ID and use ARO_HCP_TENANT_ID and ARO_HCP_MI_CLIENT_ID instead. + managedIdentityClientID := os.Getenv("ARO_HCP_MI_CLIENT_ID") + if managedIdentityClientID != "" { + options := &azidentity.ClientCertificateCredentialOptions{ ClientOptions: azcore.ClientOptions{ Cloud: cloudConfig, }, + SendCertificateChain: true, } - var err error - cred, err = azidentity.NewManagedIdentityCredential(&options) + tenantID := os.Getenv("ARO_HCP_TENANT_ID") + certPath := os.Getenv("ARO_HCP_CLIENT_CERTIFICATE_PATH") + + certData, err := os.ReadFile(certPath) + if err != nil { + return nil, fmt.Errorf("failed to read certificate file %q: %w", certPath, err) + } + + certs, key, err := azidentity.ParseCertificates(certData, nil) + if err != nil { + return nil, fmt.Errorf("failed to parse certificate data %q: %w", certPath, err) + } + + // Watch the certificate for changes; if the certificate changes, the pod will be restarted. + // This starts only one occurrence of the file watcher, which watches the file, certPath. + var fileWatcherError error + watchCertificateFileOnce.Do(func() { + if err = filewatcher.WatchFileForChanges(certPath); err != nil { + fileWatcherError = err + } + }) + if fileWatcherError != nil { + return nil, fmt.Errorf("failed to watch certificate file %q: %w", certPath, fileWatcherError) + } + + cred, err = azidentity.NewClientCertificateCredential(tenantID, managedIdentityClientID, certs, key, options) if err != nil { return nil, err } diff --git a/pkg/util/filewatcher/filewatcher.go b/pkg/util/filewatcher/filewatcher.go new file mode 100644 index 0000000000..8e79e809f1 --- /dev/null +++ b/pkg/util/filewatcher/filewatcher.go @@ -0,0 +1,65 @@ +package filewatcher + +import ( + "os" + "path/filepath" + + "github.com/fsnotify/fsnotify" + logf "github.com/openshift/cluster-ingress-operator/pkg/log" +) + +var log = logf.Logger.WithName("filewatcher") + +// WatchFileForChanges watches the file, fileToWatch, for changes. If the file contents have changed, the pod this +// function is running on will be restarted. +func WatchFileForChanges(fileToWatch string) error { + log.Info("Starting the file change watcher") + + // Update the file path to watch in case this is a symlink + fileToWatch, err := filepath.EvalSymlinks(fileToWatch) + if err != nil { + return err + } + log.Info("Watching file", "file", fileToWatch) + + // Start the file watcher to monitor file changes + go func() { + if err := checkForFileChanges(fileToWatch); err != nil { + log.Error(err, "Error checking for file changes") + } + }() + + return nil +} + +// checkForFileChanges starts a new file watcher. If the file is changed, the pod running this function will exit. +func checkForFileChanges(path string) error { + watcher, err := fsnotify.NewWatcher() + if err != nil { + return err + } + + go func() { + for { + select { + case event, ok := <-watcher.Events: + if ok && (event.Has(fsnotify.Write) || event.Has(fsnotify.Chmod) || event.Has(fsnotify.Remove)) { + log.Info("file was modified, exiting...", "file", event.Name) + os.Exit(0) + } + case err, ok := <-watcher.Errors: + if ok { + log.Error(err, "file watcher error") + } else { + log.Error(err, "failed to retrieve watcher error") + } + } + } + }() + + if err = watcher.Add(path); err != nil { + return err + } + + return nil +}