Merge pull request #1005 from openshift-cherrypick-robot/cherry-pick-1004-to-release-4.14

[release-4.14] OCPBUGS-29755: azurepathfix: fix stack hub, government and workload identity setup

Author: openshift-merge-bot[bot]

cmd/move-blobs/main.go

@@ -31,6 +31,10 @@ func main() {
 		opts.environment = "AZUREPUBLICCLOUD"
 	}
 
+	if err := createASHEnvironmentFile(opts); err != nil {
+		panic(err)
+	}
+
 	cloudConfig, err := getCloudConfig(opts.environment)
 	if err != nil {
 		panic(err)
@@ -218,6 +222,34 @@ type configOpts struct {
 	federatedTokenFile string
 	accountKey         string
 	environment        string
+	// environmentFilePath and environmentFileContents are specific
+	// for Azure Stack Hub
+	environmentFilePath     string
+	environmentFileContents string
+}
+
+func createASHEnvironmentFile(opts *configOpts) error {
+	if len(opts.environmentFilePath) == 0 || len(opts.environmentFileContents) == 0 {
+		klog.Info("Azure Stack Hub environment variables not present in current environment, skipping setup...")
+		return nil
+	}
+	f, err := os.Create(opts.environmentFilePath)
+	if err != nil {
+		return err
+	}
+
+	_, err = f.WriteString(opts.environmentFileContents)
+	if err != nil {
+		f.Close()
+		os.Remove(f.Name())
+		return err
+	}
+
+	err = f.Close()
+	if err != nil {
+		return err
+	}
+	return nil
 }
 
 func getCloudConfig(environment string) (cloud.Configuration, error) {
@@ -238,14 +270,16 @@ func getCloudConfig(environment string) (cloud.Configuration, error) {
 
 func getConfigOpts() *configOpts {
 	return &configOpts{
-		storageAccountName: strings.TrimSpace(os.Getenv("AZURE_STORAGE_ACCOUNT_NAME")),
-		containerName:      strings.TrimSpace(os.Getenv("AZURE_CONTAINER_NAME")),
-		clientID:           strings.TrimSpace(os.Getenv("AZURE_CLIENT_ID")),
-		tenantID:           strings.TrimSpace(os.Getenv("AZURE_TENANT_ID")),
-		clientSecret:       strings.TrimSpace(os.Getenv("AZURE_CLIENT_SECRET")),
-		federatedTokenFile: strings.TrimSpace(os.Getenv("AZURE_FEDERATED_TOKEN_FILE")),
-		accountKey:         strings.TrimSpace(os.Getenv("AZURE_ACCOUNTKEY")),
-		environment:        strings.TrimSpace(os.Getenv("AZURE_ENVIRONMENT")),
+		storageAccountName:      strings.TrimSpace(os.Getenv("AZURE_STORAGE_ACCOUNT_NAME")),
+		containerName:           strings.TrimSpace(os.Getenv("AZURE_CONTAINER_NAME")),
+		clientID:                strings.TrimSpace(os.Getenv("AZURE_CLIENT_ID")),
+		tenantID:                strings.TrimSpace(os.Getenv("AZURE_TENANT_ID")),
+		clientSecret:            strings.TrimSpace(os.Getenv("AZURE_CLIENT_SECRET")),
+		federatedTokenFile:      strings.TrimSpace(os.Getenv("AZURE_FEDERATED_TOKEN_FILE")),
+		accountKey:              strings.TrimSpace(os.Getenv("AZURE_ACCOUNTKEY")),
+		environment:             strings.TrimSpace(os.Getenv("AZURE_ENVIRONMENT")),
+		environmentFilePath:     strings.TrimSpace(os.Getenv("AZURE_ENVIRONMENT_FILEPATH")),
+		environmentFileContents: strings.TrimSpace(os.Getenv("AZURE_ENVIRONMENT_FILECONTENTS")),
 	}
 }
 
@@ -254,18 +288,27 @@ func getConfigOpts() *configOpts {
 // this function is basically copy of what the operator itself does,
 // as a way to ensure that it will work in the same way as the operator.
 func getClient(cloudConfig cloud.Configuration, opts *configOpts) (*container.Client, error) {
+	env, err := azure.EnvironmentFromName(opts.environment)
+	if err != nil {
+		return nil, err
+	}
 	containerURL := fmt.Sprintf(
-		"https://%s.blob.core.windows.net/%s",
+		"https://%s.blob.%s/%s",
 		opts.storageAccountName,
+		env.StorageEndpointSuffix,
 		opts.containerName,
 	)
 	var client *container.Client
+	clientOpts := azcore.ClientOptions{
+		Cloud: cloudConfig,
+	}
+
 	if len(opts.accountKey) > 0 {
 		cred, err := container.NewSharedKeyCredential(opts.storageAccountName, opts.accountKey)
 		if err != nil {
 			return nil, err
 		}
-		client, err = container.NewClientWithSharedKeyCredential(containerURL, cred, nil)
+		client, err = container.NewClientWithSharedKeyCredential(containerURL, cred, &container.ClientOptions{ClientOptions: clientOpts})
 		if err != nil {
 			return nil, err
 		}
@@ -279,7 +322,7 @@ func getClient(cloudConfig cloud.Configuration, opts *configOpts) (*container.Cl
 		if err != nil {
 			return nil, err
 		}
-		client, err = container.NewClient(containerURL, cred, nil)
+		client, err = container.NewClient(containerURL, cred, &container.ClientOptions{ClientOptions: clientOpts})
 		if err != nil {
 			return nil, err
 		}
@@ -296,16 +339,21 @@ func getClient(cloudConfig cloud.Configuration, opts *configOpts) (*container.Cl
 		if err != nil {
 			return nil, err
 		}
-		client, err = container.NewClient(containerURL, cred, nil)
+		client, err = container.NewClient(containerURL, cred, &container.ClientOptions{ClientOptions: clientOpts})
 		if err != nil {
 			return nil, err
 		}
 	} else {
-		cred, err := azidentity.NewDefaultAzureCredential(nil)
+		options := azidentity.DefaultAzureCredentialOptions{
+			ClientOptions: azcore.ClientOptions{
+				Cloud: cloudConfig,
+			},
+		}
+		cred, err := azidentity.NewDefaultAzureCredential(&options)
 		if err != nil {
 			return nil, err
 		}
-		client, err = container.NewClient(containerURL, cred, nil)
+		client, err = container.NewClient(containerURL, cred, &container.ClientOptions{ClientOptions: clientOpts})
 		if err != nil {
 			return nil, err
 		}

cmd/move-blobs/main_test.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"fmt"
+	"io"
 	"math/rand"
 	"os"
 	"strings"
@@ -74,6 +75,7 @@ func TestMoveBlobs(t *testing.T) {
 		defer client.DeleteBlob(ctx, opts.containerName, "/"+blobName, nil)
 	}
 
+	opts.environment = "AZUREPUBLICCLOUD"
 	cloudConfig, err := getCloudConfig(opts.environment)
 	if err != nil {
 		t.Fatal(err)
@@ -243,3 +245,30 @@ func TestValidation(t *testing.T) {
 		})
 	}
 }
+
+func TestStackHubEnvironmentFile(t *testing.T) {
+	path := strings.TrimSpace(os.Getenv("AZURE_ENVIRONMENT_FILEPATH"))
+	contents := strings.TrimSpace(os.Getenv("AZURE_ENVIRONMENT_FILECONTENTS"))
+	if len(path) == 0 || len(contents) == 0 {
+		t.Fatal("both AZURE_ENVIRONMENT_FILEPATH and AZURE_ENVIRONMENT_FILECONTENTS must be set")
+	}
+	opts := getConfigOpts()
+	err := createASHEnvironmentFile(opts)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(path)
+	f, err := os.Open(path)
+	if err != nil {
+		t.Fatalf("error opening file %q: %v", path, err)
+	}
+	b, err := io.ReadAll(f)
+	if err != nil {
+		t.Fatalf("error reading file %q: %v", path, err)
+	}
+	if string(b) != contents {
+		t.Logf("AZURE_ENVIRONMENT_FILECONTENTS: %s", contents)
+		t.Logf("%s: %s", path, string(b))
+		t.Fatalf("file contents differed from AZURE_ENVIRONMENT_FILECONTENTS")
+	}
+}

pkg/operator/azurepathfixcontroller.go

@@ -3,6 +3,7 @@ package operator
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	batchv1 "k8s.io/api/batch/v1"
@@ -45,6 +46,7 @@ type AzurePathFixController struct {
 	podLister                 corev1listers.PodNamespaceLister
 	infrastructureLister      configlisters.InfrastructureLister
 	proxyLister               configlisters.ProxyLister
+	openshiftConfigLister     corev1listers.ConfigMapNamespaceLister
 	kubeconfig                *restclient.Config
 
 	cachesToSync []cache.InformerSynced
@@ -60,6 +62,7 @@ func NewAzurePathFixController(
 	infrastructureInformer configv1informers.InfrastructureInformer,
 	secretInformer corev1informers.SecretInformer,
 	proxyInformer configv1informers.ProxyInformer,
+	openshiftConfigInformer corev1informers.ConfigMapInformer,
 	podInformer corev1informers.PodInformer,
 ) (*AzurePathFixController, error) {
 	c := &AzurePathFixController{
@@ -71,6 +74,7 @@ func NewAzurePathFixController(
 		secretLister:              secretInformer.Lister().Secrets(defaults.ImageRegistryOperatorNamespace),
 		podLister:                 podInformer.Lister().Pods(defaults.ImageRegistryOperatorNamespace),
 		proxyLister:               proxyInformer.Lister(),
+		openshiftConfigLister:     openshiftConfigInformer.Lister().ConfigMaps(defaults.OpenShiftConfigNamespace),
 		kubeconfig:                kubeconfig,
 		queue:                     workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "AzurePathFixController"),
 	}
@@ -105,6 +109,11 @@ func NewAzurePathFixController(
 	}
 	c.cachesToSync = append(c.cachesToSync, proxyInformer.Informer().HasSynced)
 
+	if _, err := openshiftConfigInformer.Informer().AddEventHandler(c.eventHandler()); err != nil {
+		return nil, err
+	}
+	c.cachesToSync = append(c.cachesToSync, openshiftConfigInformer.Informer().HasSynced)
+
 	// bootstrap the job if it doesn't exist
 	c.queue.Add("instance")
 
@@ -168,6 +177,7 @@ func (c *AzurePathFixController) sync() error {
 	if err != nil {
 		return err
 	}
+
 	azureStorage := imageRegistryConfig.Status.Storage.Azure
 	if azureStorage == nil || len(azureStorage.AccountName) == 0 {
 		return fmt.Errorf("storage account not yet provisioned")
@@ -176,12 +186,19 @@ func (c *AzurePathFixController) sync() error {
 		return fmt.Errorf("storage container not yet provisioned")
 	}
 
+	// the move-blobs cmd does not work on Azure Stack Hub. Users on ASH
+	// will have to copy the blobs on their own using something like az copy.
+	if strings.EqualFold(azureStorage.CloudName, "AZURESTACKCLOUD") {
+		return nil
+	}
+
 	gen := resource.NewGeneratorAzurePathFixJob(
 		c.jobLister,
 		c.batchClient,
 		c.secretLister,
 		c.infrastructureLister,
 		c.proxyLister,
+		c.openshiftConfigLister,
 		imageRegistryConfig,
 		c.kubeconfig,
 	)

pkg/operator/starter.go

@@ -177,6 +177,7 @@ func RunOperator(ctx context.Context, kubeconfig *restclient.Config) error {
 		configInformers.Config().V1().Infrastructures(),
 		kubeInformers.Core().V1().Secrets(),
 		configInformers.Config().V1().Proxies(),
+		kubeInformersForOpenShiftConfig.Core().V1().ConfigMaps(),
 		kubeInformers.Core().V1().Pods(),
 	)
 	if err != nil {