Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/v1beta1/azureclusteridentity_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
20 changes: 15 additions & 5 deletions azure/scope/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import (
"context"
"os"
"reflect"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
Expand Down Expand Up @@ -127,11 +128,20 @@
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
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)

Check warning on line 142 in azure/scope/identity.go

View check run for this annotation

Codecov / codecov/patch

azure/scope/identity.go#L135-L142

Added lines #L135 - L142 were not covered by tests
}
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")
}
Expand Down Expand Up @@ -200,7 +210,7 @@
// 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
Expand Down
28 changes: 22 additions & 6 deletions azure/scope/identity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,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",
Expand Down Expand Up @@ -302,9 +301,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{
Expand All @@ -316,6 +313,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{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
43 changes: 27 additions & 16 deletions controllers/asosecret_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import (
"context"
"fmt"
"os"

asoconfig "github.com/Azure/azure-service-operator/v2/pkg/common/config"
"github.com/pkg/errors"
Expand Down Expand Up @@ -287,23 +288,33 @@
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 != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you able to add a test case or two to asosecret_controller_test.go for this case when CertPath is specified?

Copy link
Contributor Author

@bryan-cox bryan-cox Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I added a test which found a minor error. Service Principal with Certificate was expected to have a client secret here. That's been fixed in this PR. Client Secret isn't needed with using Certificate (see this in the Azure SDK as an example).

certsContent, err := os.ReadFile(identity.Spec.CertPath)
if err != nil {
return nil, errors.Wrap(err, "failed to read certificate file")
}

Check warning on line 295 in controllers/asosecret_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/asosecret_controller.go#L294-L295

Added lines #L294 - L295 were not covered by tests

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")
}

Check warning on line 309 in controllers/asosecret_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/asosecret_controller.go#L308-L309

Added lines #L308 - L309 were not covered by tests

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"]

Check warning on line 316 in controllers/asosecret_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/asosecret_controller.go#L314-L316

Added lines #L314 - L316 were not covered by tests
}
}
return newASOSecret, nil
}
25 changes: 25 additions & 0 deletions controllers/asosecret_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,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{
Expand Down
18 changes: 18 additions & 0 deletions docs/book/src/topics/identities.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,24 @@ data:
password: PASSWORD
```

Alternatively, the path to a certificate can be specified instead of the k8s secret:

```yaml
apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: AzureClusterIdentity
metadata:
name: example-identity
namespace: default
spec:
type: ServicePrincipalCertificate
tenantID: <azure-tenant-id>
clientID: <client-id-of-SP-identity>
certPath: <path-to-the-cert>
allowedNamespaces:
list:
- <cluster-namespace>
```

## User-Assigned Managed Identity

<aside class="note">
Expand Down
47 changes: 47 additions & 0 deletions test/setup/certificate
Original file line number Diff line number Diff line change
@@ -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-----