Skip to content

Commit

Permalink
Merge pull request #1004 from openshift-cherrypick-robot/cherry-pick-…
Browse files Browse the repository at this point in the history 
…1003-to-release-4.15

[release-4.15] OCPBUGS-29638: azurepathfix: fix stack hub, government and workload identity setup
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] committed Feb 21, 2024
2 parents 95e4fb4 + 0755c32 commit 5da92bd
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 39 deletions.
76 changes: 62 additions & 14 deletions cmd/move-blobs/main.go
View file
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand All @@ -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")),
}
}

Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
29 changes: 29 additions & 0 deletions cmd/move-blobs/main_test.go
View file
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"fmt"
"io"
"math/rand"
"os"
"strings"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
}
}
17 changes: 17 additions & 0 deletions pkg/operator/azurepathfixcontroller.go
View file
Expand Up @@ -3,6 +3,7 @@ package operator
import (
"context"
"fmt"
"strings"
"time"

batchv1 "k8s.io/api/batch/v1"
Expand Down Expand Up @@ -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
Expand All @@ -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{
Expand All @@ -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"),
}
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")
Expand All @@ -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,
)
Expand Down
1 change: 1 addition & 0 deletions pkg/operator/starter.go
View file
Expand Up @@ -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 {
Expand Down

0 comments on commit 5da92bd

Please sign in to comment.