ARO-4373 additional unit tests and comments resolution

pkg/cluster/deploybaseresources.go

@@ -46,7 +46,7 @@ func (m *manager) createOIDC(ctx context.Context) error {
 	blobContainerName := env.OIDCBlobContainerPrefix + m.doc.ID
 
 	publicAccess := azstorage.PublicAccessNone
-	// Public access on OIDC Container needed for development environments because of no AFD avaialbility
+	// Public access on OIDC Container needed for development environments because of no AFD availability
 	if m.env.IsLocalDevelopmentMode() {
 		publicAccess = azstorage.PublicAccessBlob
 	}

pkg/cluster/deploybaseresources_test.go

@@ -1557,6 +1557,34 @@ func TestCreateOIDC(t *testing.T) {
 			wantBoundServiceAccountSigningKey: false,
 			wantErr:                           "generic error",
 		},
+		{
+			name: "Fail - OIDCBuilder fails to populate OIDC blob",
+			oc: &api.OpenShiftClusterDocument{
+				Key: strings.ToLower(resourceID),
+				ID:  clusterID,
+				OpenShiftCluster: &api.OpenShiftCluster{
+					Properties: api.OpenShiftClusterProperties{
+						ClusterProfile: api.ClusterProfile{
+							ResourceGroupID: resourceGroup,
+						},
+						PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{},
+					},
+				},
+			},
+			mocks: func(blob *mock_azblob.MockManager, env *mock_env.MockInterface, azblobClient *mock_azblob.MockAZBlobClient) {
+				env.EXPECT().OIDCStorageAccountName().AnyTimes().Return(oidcStorageAccountName)
+				env.EXPECT().IsLocalDevelopmentMode().Return(false)
+				env.EXPECT().ResourceGroup().Return(resourceGroupName)
+				env.EXPECT().Environment().Return(&azureclient.PublicCloud)
+				env.EXPECT().OIDCEndpoint().Return(afdEndpoint)
+				blob.EXPECT().CreateBlobContainer(gomock.Any(), resourceGroupName, oidcStorageAccountName, gomock.Any(), azstorage.PublicAccessNone).Return(nil)
+				azblobClient.EXPECT().UploadBuffer(gomock.Any(), "", oidcbuilder.DiscoveryDocumentKey, gomock.Any()).Return(nil)
+				azblobClient.EXPECT().UploadBuffer(gomock.Any(), "", oidcbuilder.JWKSKey, gomock.Any()).Return(errors.New("generic error"))
+				blob.EXPECT().GetAZBlobClient(gomock.Any(), &azblob.ClientOptions{}).Return(azblobClient, nil)
+			},
+			wantBoundServiceAccountSigningKey: false,
+			wantErr:                           "generic error",
+		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			controller := gomock.NewController(t)

pkg/util/azblob/manager.go

@@ -39,23 +39,16 @@ func NewManager(environment *azureclient.AROEnvironment, subscriptionID string,
 }
 
 func (m *manager) CreateBlobContainer(ctx context.Context, resourceGroup string, accountName string, containerName string, publicAccess azstorage.PublicAccess) error {
-	needToCreateBlobContainer := false
-
 	_, err := m.blobContainer.Get(
 		ctx,
 		resourceGroup,
 		accountName,
 		containerName,
 		&azstorage.BlobContainersClientGetOptions{},
 	)
-	if err != nil {
-		if !bloberror.HasCode(err, bloberror.ContainerNotFound) {
-			return err
-		}
-		needToCreateBlobContainer = true
-	}
-
-	if !needToCreateBlobContainer {
+	if err != nil && !bloberror.HasCode(err, bloberror.ContainerNotFound) {
+		return err
+	} else if err == nil {
 		return nil
 	}
 

pkg/util/oidcbuilder/jwks.go

@@ -31,10 +31,10 @@ func CreateKeyPair() (encPrivateKey []byte, encPublicKey []byte, err error) {
 		Bytes:   x509.MarshalPKCS1PrivateKey(privateKey),
 	})
 
-	// Generate public key from private keypair
+	// Serialize public key into a byte array to prepare to store it in the OIDC storage blob
 	pubKeyBytes, err := x509.MarshalPKIXPublicKey(&privateKey.PublicKey)
 	if err != nil {
-		return nil, nil, errors.Wrapf(err, "failed to generate public key from private")
+		return nil, nil, errors.Wrapf(err, "failed to serialize public key")
 	}
 	encodedPublicKey := pem.EncodeToMemory(&pem.Block{
 		Type:    "PUBLIC KEY",

pkg/util/oidcbuilder/jwks_test.go

@@ -0,0 +1,60 @@
+package oidcbuilder
+
+import (
+	"crypto/rand"
+	"crypto/rsa"
+	"testing"
+
+	"github.com/golang/mock/gomock"
+
+	mock_azblob "github.com/Azure/ARO-RP/pkg/util/mocks/azblob"
+	utilerror "github.com/Azure/ARO-RP/test/util/error"
+)
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+func TestKeyIDFromPublicKey(t *testing.T) {
+	privateKey, err := rsa.GenerateKey(rand.Reader, 4096)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	keyID, err := keyIDFromPublicKey(&privateKey.PublicKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	invalidPublicKey := rsa.PublicKey{}
+
+	for _, tt := range []struct {
+		name      string
+		mocks     func(*mock_azblob.MockAZBlobClient)
+		publicKey interface{}
+		wantkid   string
+		wantErr   string
+	}{
+		{
+			name:      "Success",
+			publicKey: &privateKey.PublicKey,
+			wantkid:   keyID,
+		},
+		{
+			name:      "Failed to serialize public key",
+			publicKey: &invalidPublicKey,
+			wantErr:   "Failed to serialize public key to DER format: asn1: structure error: empty integer",
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			controller := gomock.NewController(t)
+			defer controller.Finish()
+
+			kid, err := keyIDFromPublicKey(tt.publicKey)
+			utilerror.AssertErrorMessage(t, err, tt.wantErr)
+
+			if tt.wantkid != kid {
+				t.Fatalf("Expected KeyID and returned KeyID doesn't match")
+			}
+		})
+	}
+}

pkg/util/oidcbuilder/oidcbuilder_test.go

@@ -2,6 +2,8 @@ package oidcbuilder
 
 import (
 	"context"
+	"crypto/ecdsa"
+	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/x509"
@@ -40,6 +42,14 @@ func TestEnsureOIDCDocs(t *testing.T) {
 		Bytes:   pubKeyBytes,
 	})
 
+	nonRSAPrivateKey, _ := ecdsa.GenerateKey(elliptic.P384(), rand.Reader)
+	nonRSAPubKeyBytes, err := x509.MarshalPKIXPublicKey(&nonRSAPrivateKey.PublicKey)
+	nonRSAEncodedPublicKey := pem.EncodeToMemory(&pem.Block{
+		Type:    "PUBLIC KEY",
+		Headers: nil,
+		Bytes:   nonRSAPubKeyBytes,
+	})
+
 	invalidKey := []byte("Invalid Key")
 
 	for _, tt := range []struct {
@@ -118,6 +128,16 @@ func TestEnsureOIDCDocs(t *testing.T) {
 			},
 			wantErr: "generic error",
 		},
+		{
+			name: "Fail - Public key is not of type RSA",
+			oidcbuilder: &OIDCBuilder{
+				privateKey:       priKey,
+				publicKey:        nonRSAEncodedPublicKey,
+				blobContainerURL: blobContainerURL,
+				endpointURL:      endpointURL,
+			},
+			wantErr: "Public key is not of type RSA",
+		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			controller := gomock.NewController(t)