Merge pull request #323 from bryan-cox/aro-hcp-cert-auth

OCPBUGS-42434: Add the ability to auth via certs without storing them in etcd secret

Author: openshift-merge-bot[bot]

api/v1beta1/azureclusteridentity_types.go

@@ -59,6 +59,9 @@ type AzureClusterIdentitySpec struct {
 	// ClientSecret is a secret reference which should contain either a Service Principal password or certificate secret.
 	// +optional
 	ClientSecret corev1.SecretReference `json:"clientSecret,omitempty"`
+	// CertPath is the path where certificates exist. When set, it takes precedence over ClientSecret for types that use certs like ServicePrincipalCertificate.
+	// +optional
+	CertPath string `json:"certPath,omitempty"`
 	// TenantID is the service principal primary tenant id.
 	TenantID string `json:"tenantID"`
 	// AllowedNamespaces is used to identify the namespaces the clusters are allowed to use the identity from.

azure/scope/identity.go

@@ -18,6 +18,7 @@ package scope
 
 import (
 	"context"
+	"os"
 	"reflect"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
@@ -116,11 +117,23 @@ func (p *AzureCredentialsProvider) GetTokenCredential(ctx context.Context, resou
 		cred, authErr = azidentity.NewClientSecretCredential(p.GetTenantID(), p.Identity.Spec.ClientID, clientSecret, &options)
 
 	case infrav1.ServicePrincipalCertificate:
-		clientSecret, err := p.GetClientSecret(ctx)
-		if err != nil {
-			return nil, errors.Wrap(err, "failed to get client secret")
+		var (
+			certsContent []byte
+			err          error
+		)
+		if p.Identity.Spec.CertPath != "" {
+			certsContent, err = os.ReadFile(p.Identity.Spec.CertPath)
+			if err != nil {
+				return nil, errors.Wrap(err, "failed to read certificate file")
+			}
+		} else {
+			clientSecret, err := p.GetClientSecret(ctx)
+			if err != nil {
+				return nil, errors.Wrap(err, "failed to get client secret")
+			}
+			certsContent = []byte(clientSecret)
 		}
-		certs, key, err := azidentity.ParseCertificates([]byte(clientSecret), nil)
+		certs, key, err := azidentity.ParseCertificates(certsContent, nil)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to parse certificate data")
 		}
@@ -182,7 +195,7 @@ func (p *AzureCredentialsProvider) Type() infrav1.IdentityType {
 // This does not include managed identities.
 func (p *AzureCredentialsProvider) hasClientSecret() bool {
 	switch p.Identity.Spec.Type {
-	case infrav1.ServicePrincipal, infrav1.ManualServicePrincipal, infrav1.ServicePrincipalCertificate:
+	case infrav1.ServicePrincipal, infrav1.ManualServicePrincipal:
 		return true
 	default:
 		return false

azure/scope/identity_test.go

@@ -162,11 +162,10 @@ func TestHasClientSecret(t *testing.T) {
 			name: "service principal with certificate",
 			identity: &infrav1.AzureClusterIdentity{
 				Spec: infrav1.AzureClusterIdentitySpec{
-					Type:         infrav1.ServicePrincipalCertificate,
-					ClientSecret: corev1.SecretReference{Name: "my-client-secret"},
+					Type: infrav1.ServicePrincipalCertificate,
 				},
 			},
-			want: true,
+			want: false,
 		},
 		{
 			name: "manual service principal",
@@ -301,9 +300,7 @@ func TestGetTokenCredential(t *testing.T) {
 				Spec: infrav1.AzureClusterIdentitySpec{
 					Type:     infrav1.ServicePrincipalCertificate,
 					TenantID: fakeTenantID,
-					ClientSecret: corev1.SecretReference{
-						Name: "test-identity-secret",
-					},
+					CertPath: "../../test/setup/certificate",
 				},
 			},
 			secret: &corev1.Secret{
@@ -315,6 +312,25 @@ func TestGetTokenCredential(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "service principal certificate with certificate filepath",
+			cluster: &infrav1.AzureCluster{
+				Spec: infrav1.AzureClusterSpec{
+					AzureClusterClassSpec: infrav1.AzureClusterClassSpec{
+						IdentityRef: &corev1.ObjectReference{
+							Kind: infrav1.AzureClusterIdentityKind,
+						},
+					},
+				},
+			},
+			identity: &infrav1.AzureClusterIdentity{
+				Spec: infrav1.AzureClusterIdentitySpec{
+					Type:     infrav1.ServicePrincipalCertificate,
+					TenantID: fakeTenantID,
+					CertPath: "../../test/setup/certificate",
+				},
+			},
+		},
 		{
 			name: "user-assigned identity",
 			cluster: &infrav1.AzureCluster{

config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusteridentities.yaml

@@ -123,6 +123,11 @@ spec:
                     type: object
                     x-kubernetes-map-type: atomic
                 type: object
+              certPath:
+                description: CertPath is the path where certificates exist. When set,
+                  it takes precedence over ClientSecret for types that use certs like
+                  ServicePrincipalCertificate.
+                type: string
               clientID:
                 description: |-
                   ClientID is the service principal client ID.

controllers/asosecret_controller.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"os"
 
 	asoconfig "github.com/Azure/azure-service-operator/v2/pkg/common/config"
 	"github.com/pkg/errors"
@@ -288,23 +289,33 @@ func (asos *ASOSecretReconciler) createSecretFromClusterIdentity(ctx context.Con
 		return newASOSecret, nil
 	}
 
-	// Fetch identity secret, if it exists
-	key = types.NamespacedName{
-		Namespace: identity.Spec.ClientSecret.Namespace,
-		Name:      identity.Spec.ClientSecret.Name,
-	}
-	identitySecret := &corev1.Secret{}
-	err := asos.Get(ctx, key, identitySecret)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to fetch AzureClusterIdentity secret")
-	}
+	if identity.Spec.CertPath != "" {
+		certsContent, err := os.ReadFile(identity.Spec.CertPath)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to read certificate file")
+		}
 
-	switch identity.Spec.Type {
-	case infrav1.ServicePrincipal, infrav1.ManualServicePrincipal:
-		newASOSecret.Data[asoconfig.AzureClientSecret] = identitySecret.Data[scope.AzureSecretKey]
-	case infrav1.ServicePrincipalCertificate:
-		newASOSecret.Data[asoconfig.AzureClientCertificate] = identitySecret.Data["certificate"]
-		newASOSecret.Data[asoconfig.AzureClientCertificatePassword] = identitySecret.Data["password"]
+		newASOSecret.Data[asoconfig.AzureClientCertificate] = certsContent
+		newASOSecret.Data[asoconfig.AzureClientCertificatePassword] = []byte{}
+	} else {
+		// Fetch identity secret, if it exists
+		key = types.NamespacedName{
+			Namespace: identity.Spec.ClientSecret.Namespace,
+			Name:      identity.Spec.ClientSecret.Name,
+		}
+		identitySecret := &corev1.Secret{}
+		err := asos.Get(ctx, key, identitySecret)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to fetch AzureClusterIdentity secret")
+		}
+
+		switch identity.Spec.Type {
+		case infrav1.ServicePrincipal, infrav1.ManualServicePrincipal:
+			newASOSecret.Data[asoconfig.AzureClientSecret] = identitySecret.Data[scope.AzureSecretKey]
+		case infrav1.ServicePrincipalCertificate:
+			newASOSecret.Data[asoconfig.AzureClientCertificate] = identitySecret.Data["certificate"]
+			newASOSecret.Data[asoconfig.AzureClientCertificatePassword] = identitySecret.Data["password"]
+		}
 	}
 	return newASOSecret, nil
 }

controllers/asosecret_controller_test.go

@@ -134,6 +134,31 @@ func TestASOSecretReconcile(t *testing.T) {
 				}
 			}),
 		},
+		"should reconcile normally for AzureManagedControlPlane with IdentityRef configured of type Service Principal with Certificate": {
+			clusterName: defaultAzureManagedControlPlane.Name,
+			objects: []runtime.Object{
+				getASOAzureManagedControlPlane(func(c *infrav1.AzureManagedControlPlane) {
+					c.Spec.IdentityRef = &corev1.ObjectReference{
+						Name:      "my-azure-cluster-identity",
+						Namespace: "default",
+					}
+				}),
+				getASOAzureClusterIdentity(func(identity *infrav1.AzureClusterIdentity) {
+					identity.Spec.Type = infrav1.ServicePrincipalCertificate
+					identity.Spec.CertPath = "../test/setup/certificate"
+				}),
+				defaultCluster,
+			},
+			asoSecret: getASOSecret(defaultAzureManagedControlPlane, func(s *corev1.Secret) {
+				s.Data = map[string][]byte{
+					"AZURE_SUBSCRIPTION_ID":             []byte("fooSubscription"),
+					"AZURE_TENANT_ID":                   []byte("fooTenant"),
+					"AZURE_CLIENT_ID":                   []byte("fooClient"),
+					"AZURE_CLIENT_CERTIFICATE_PASSWORD": []byte(""),
+					"AZURE_CLIENT_CERTIFICATE":          []byte("-----BEGIN PRIVATE KEY-----\nMIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDjrdEr9P0TaUES\ndspE6cyo22NU8yhRrbYlV9VH2vWvnPsThXcxhnd+cUqdNEBswhwgFlUQcg/eSVxw\nrr+3nh+bFTZWPcY+1LQYxfpKGsrCXQfB82LDJIZDX4gHYrWf3Z272jXN1XeFAKti\nwDKgDXXuPH7r5lH7vC3RXeAffqLwQJhZf+NoHNtv9MH9IdUkQfmDFZtI/CQzCrb6\n+vOS6EmUD/Q2FNHBzgxCguGqgNyBcQbxJ9Qng+ZjIFuhGYXJlsyRUtexyzTR5/v0\nVNK8UsZgRBFhXqrBv/RoCCG+xVJYtmd0QsrvNzDqG6QnjUB21zVXqzKEkW2gRtjX\ncw4vYQehAgMBAAECggEAS6xtjg0nAokk0jS+ZOpKlkMZAFaza3ZvyHipkHDz4PMt\ntl7Rb5oQZGvWT2rbEOrxey7BBi7LHGhIu8ExQp/hRGPoBAETP7XlyCghWPkPtEtE\ndU/mXxLoN0NszHuf/2si7pmH8YqGZ6QB0tgr22ut60mbK+AJFsEEf4aSpBUspepJ\n2800sQHsqPE6L6kYkfZ2GRRY1V9vUrYEODKZpWzMhN3UA9nAKH9PB6xvP2OdyMNh\nhKgmUUMNIFtwr8pZlJn60cf0UrWrc5CvqQLuaGYlzDgUQGV4JEVjqm9F6lMfEPUw\neN70MVe1pcLeLq2rGCVWU3gakh/HvJqlR/sa546HgwKBgQDyf1vkyX4w5sboi6DJ\ncl5dMULtMMRpB1OaMFVOJjI9gZJ8mCdRjqXdYo5aS2KIqxie8tGG9+SohxDAWl4t\nlSUtDsE44fSmILqC5zIawNRQnnkv0X8LwmYu0Qd7YAjJMlLTWyDRsjD9XRq4nsR+\nmJVwrt85iSpS5UFyryEzPbFj0wKBgQDwWzraeN0Eccf1iIYmQsYy+yMEAlHNR5yi\ngPXuAhSybv2JReRhdUb39hLr/LvKw0ZeXiLWXmYUGpbyzPyXIm0s+PL3LWl65GTF\nl+cfV5wfAdDkk6rAdEPEE2pxN85ChyaPYPoYr0ohmV97VQcYc5FqY+j1tM6R1RDt\n/fWBSa8iOwKBgQCpa1dtWWTDj4gqUdrswu2wmEkU47xlUIwVLm164u64z/zi9X6K\n2WmCaWfhJ8fYigjyi9zdOfXT1EFc0gX4PLozZ5qRPjQpmLYV3KbB0DTFemJaiTgE\npDW1wa5DgQ3CW1lIduNP/fmCGfkgQTQw6jOF/XbRgMZEEg2OrVI5tYFopwKBgER9\niqjEth5VGejCjY+LiZTvcUvsKUk4tc6stueqmiE6dW7PhsOqup1f9oZej1i5Cm1L\nn9u8LJRf+1GWzgd3HOsqyXlb7GnDeV/A6HBK88b2KoNn/Mk4mDLgYX1/rHvSrU9A\nECRGlvY6ETZAxXPXQsGxVKnnatGtiFR5AKNlzs0PAoGAa5+X+DUqGh9aE5ID3wrv\njkjxQ2KLFJCNSq8f9GSuvpvgXstHh6wKoM6vMwIShjgXuURH8Ub4uhRsWnxMildF\n7EE+QaWU9jnCm2HQYArfXrAWw6DBudiSkBqgKc6HjDHun5fXlYUo8UesNMQOrg7b\nbydQZ5/4V/1oSWPETk7jSr0=\n-----END PRIVATE KEY-----\n-----BEGIN CERTIFICATE-----\nMIIDCTCCAfGgAwIBAgIUFSntEn+Tv6HM2xJReECJpJcC7iUwDQYJKoZIhvcNAQEL\nBQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MB4XDTI0MDEwODE5NTQxNFoXDTM0MDEw\nNTE5NTQxNFowFDESMBAGA1UEAwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0BAQEF\nAAOCAQ8AMIIBCgKCAQEA463RK/T9E2lBEnbKROnMqNtjVPMoUa22JVfVR9r1r5z7\nE4V3MYZ3fnFKnTRAbMIcIBZVEHIP3klccK6/t54fmxU2Vj3GPtS0GMX6ShrKwl0H\nwfNiwySGQ1+IB2K1n92du9o1zdV3hQCrYsAyoA117jx+6+ZR+7wt0V3gH36i8ECY\nWX/jaBzbb/TB/SHVJEH5gxWbSPwkMwq2+vrzkuhJlA/0NhTRwc4MQoLhqoDcgXEG\n8SfUJ4PmYyBboRmFyZbMkVLXscs00ef79FTSvFLGYEQRYV6qwb/0aAghvsVSWLZn\ndELK7zcw6hukJ41Adtc1V6syhJFtoEbY13MOL2EHoQIDAQABo1MwUTAdBgNVHQ4E\nFgQUfry/KDtamwMlRQsFPbBhzdv2U5cwHwYDVR0jBBgwFoAUfry/KDtamwMlRQsF\nPbBhzdv2U5cwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAyYst\nVvewKRRpuYRWc4XG6WnYphUdyZLMoIlq0syZ1aj6YbqoK9NMHAYEnCvSov6zIZOa\ntrhuUcf9GFz5e0iJ2zIlDc312Iwsv41xiC/bs16kEn8Yf/SujEXasj7vmA3HrFWf\nwZTH/yFL5azo/f+lA1Q28YwqFpHmle0y0O53Uth4p0tmwlnu+CrO9fHp3kTlb7fD\n6mqfk9Nrt8tOC4aHYDoqtYUgZhx58xsHMOTetKeRlp8HMF9oROtriz4nYm6IhTwo\n5k1A13S3BjaxkZCyPXCgXssuXagNLasrr5Qq+Vgdb/nDhVehV8+Z4J0Ynzy9MZsE\nH1N1NfMtsA+PEqtPXA==\n-----END CERTIFICATE-----\n"),
+				}
+			}),
+		},
 		"should reconcile normally for AzureCluster with an IdentityRef of type WorkloadIdentity": {
 			clusterName: defaultAzureCluster.Name,
 			objects: []runtime.Object{

test/setup/certificate

@@ -0,0 +1,47 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDjrdEr9P0TaUES
+dspE6cyo22NU8yhRrbYlV9VH2vWvnPsThXcxhnd+cUqdNEBswhwgFlUQcg/eSVxw
+rr+3nh+bFTZWPcY+1LQYxfpKGsrCXQfB82LDJIZDX4gHYrWf3Z272jXN1XeFAKti
+wDKgDXXuPH7r5lH7vC3RXeAffqLwQJhZf+NoHNtv9MH9IdUkQfmDFZtI/CQzCrb6
++vOS6EmUD/Q2FNHBzgxCguGqgNyBcQbxJ9Qng+ZjIFuhGYXJlsyRUtexyzTR5/v0
+VNK8UsZgRBFhXqrBv/RoCCG+xVJYtmd0QsrvNzDqG6QnjUB21zVXqzKEkW2gRtjX
+cw4vYQehAgMBAAECggEAS6xtjg0nAokk0jS+ZOpKlkMZAFaza3ZvyHipkHDz4PMt
+tl7Rb5oQZGvWT2rbEOrxey7BBi7LHGhIu8ExQp/hRGPoBAETP7XlyCghWPkPtEtE
+dU/mXxLoN0NszHuf/2si7pmH8YqGZ6QB0tgr22ut60mbK+AJFsEEf4aSpBUspepJ
+2800sQHsqPE6L6kYkfZ2GRRY1V9vUrYEODKZpWzMhN3UA9nAKH9PB6xvP2OdyMNh
+hKgmUUMNIFtwr8pZlJn60cf0UrWrc5CvqQLuaGYlzDgUQGV4JEVjqm9F6lMfEPUw
+eN70MVe1pcLeLq2rGCVWU3gakh/HvJqlR/sa546HgwKBgQDyf1vkyX4w5sboi6DJ
+cl5dMULtMMRpB1OaMFVOJjI9gZJ8mCdRjqXdYo5aS2KIqxie8tGG9+SohxDAWl4t
+lSUtDsE44fSmILqC5zIawNRQnnkv0X8LwmYu0Qd7YAjJMlLTWyDRsjD9XRq4nsR+
+mJVwrt85iSpS5UFyryEzPbFj0wKBgQDwWzraeN0Eccf1iIYmQsYy+yMEAlHNR5yi
+gPXuAhSybv2JReRhdUb39hLr/LvKw0ZeXiLWXmYUGpbyzPyXIm0s+PL3LWl65GTF
+l+cfV5wfAdDkk6rAdEPEE2pxN85ChyaPYPoYr0ohmV97VQcYc5FqY+j1tM6R1RDt
+/fWBSa8iOwKBgQCpa1dtWWTDj4gqUdrswu2wmEkU47xlUIwVLm164u64z/zi9X6K
+2WmCaWfhJ8fYigjyi9zdOfXT1EFc0gX4PLozZ5qRPjQpmLYV3KbB0DTFemJaiTgE
+pDW1wa5DgQ3CW1lIduNP/fmCGfkgQTQw6jOF/XbRgMZEEg2OrVI5tYFopwKBgER9
+iqjEth5VGejCjY+LiZTvcUvsKUk4tc6stueqmiE6dW7PhsOqup1f9oZej1i5Cm1L
+n9u8LJRf+1GWzgd3HOsqyXlb7GnDeV/A6HBK88b2KoNn/Mk4mDLgYX1/rHvSrU9A
+ECRGlvY6ETZAxXPXQsGxVKnnatGtiFR5AKNlzs0PAoGAa5+X+DUqGh9aE5ID3wrv
+jkjxQ2KLFJCNSq8f9GSuvpvgXstHh6wKoM6vMwIShjgXuURH8Ub4uhRsWnxMildF
+7EE+QaWU9jnCm2HQYArfXrAWw6DBudiSkBqgKc6HjDHun5fXlYUo8UesNMQOrg7b
+bydQZ5/4V/1oSWPETk7jSr0=
+-----END PRIVATE KEY-----
+-----BEGIN CERTIFICATE-----
+MIIDCTCCAfGgAwIBAgIUFSntEn+Tv6HM2xJReECJpJcC7iUwDQYJKoZIhvcNAQEL
+BQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MB4XDTI0MDEwODE5NTQxNFoXDTM0MDEw
+NTE5NTQxNFowFDESMBAGA1UEAwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0BAQEF
+AAOCAQ8AMIIBCgKCAQEA463RK/T9E2lBEnbKROnMqNtjVPMoUa22JVfVR9r1r5z7
+E4V3MYZ3fnFKnTRAbMIcIBZVEHIP3klccK6/t54fmxU2Vj3GPtS0GMX6ShrKwl0H
+wfNiwySGQ1+IB2K1n92du9o1zdV3hQCrYsAyoA117jx+6+ZR+7wt0V3gH36i8ECY
+WX/jaBzbb/TB/SHVJEH5gxWbSPwkMwq2+vrzkuhJlA/0NhTRwc4MQoLhqoDcgXEG
+8SfUJ4PmYyBboRmFyZbMkVLXscs00ef79FTSvFLGYEQRYV6qwb/0aAghvsVSWLZn
+dELK7zcw6hukJ41Adtc1V6syhJFtoEbY13MOL2EHoQIDAQABo1MwUTAdBgNVHQ4E
+FgQUfry/KDtamwMlRQsFPbBhzdv2U5cwHwYDVR0jBBgwFoAUfry/KDtamwMlRQsF
+PbBhzdv2U5cwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAyYst
+VvewKRRpuYRWc4XG6WnYphUdyZLMoIlq0syZ1aj6YbqoK9NMHAYEnCvSov6zIZOa
+trhuUcf9GFz5e0iJ2zIlDc312Iwsv41xiC/bs16kEn8Yf/SujEXasj7vmA3HrFWf
+wZTH/yFL5azo/f+lA1Q28YwqFpHmle0y0O53Uth4p0tmwlnu+CrO9fHp3kTlb7fD
+6mqfk9Nrt8tOC4aHYDoqtYUgZhx58xsHMOTetKeRlp8HMF9oROtriz4nYm6IhTwo
+5k1A13S3BjaxkZCyPXCgXssuXagNLasrr5Qq+Vgdb/nDhVehV8+Z4J0Ynzy9MZsE
+H1N1NfMtsA+PEqtPXA==
+-----END CERTIFICATE-----