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
9 changes: 6 additions & 3 deletions api/v1beta1/azureclusteridentity_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,20 @@ type AllowedNamespaces struct {

// AzureClusterIdentitySpec defines the parameters that are used to create an AzureIdentity.
type AzureClusterIdentitySpec struct {
// UserAssignedMSI or Service Principal
// Type is the type of Azure Identity used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Type is the type of Azure Identity used.
// IdentityType is the type of Azure Identity used.

Although since this field is just type in its JSON representation, making this a proper Go comment here will create a naming discrepancy in the generated CRD description. I still think that would be more correct, but I am not invested in this change. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Type is the name of the field too... IdentityType is the type of the field (this is confusing!) :D

So I think Type is more correct

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right--so hard to read when things "stutter" like that.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤯

// ServicePrincipal, ServicePrincipalCertificate, or ManualServicePrincipal.
Type IdentityType `json:"type"`
// User assigned MSI resource id.
// ResourceID is the Azure resource ID for the User Assigned MSI resource.
// Not currently supported.
// +optional
ResourceID string `json:"resourceID,omitempty"`
// ClientID is the service principal client ID.
// Both User Assigned MSI and SP can use this field.
ClientID string `json:"clientID"`
// ClientSecret is a secret reference which should contain either a Service Principal password or certificate secret.
// +optional
ClientSecret corev1.SecretReference `json:"clientSecret,omitempty"`
// Service principal primary tenant id.
// 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.
// Namespaces can be selected either using an array of namespaces or with label selector.
Expand Down
7 changes: 5 additions & 2 deletions api/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,18 +422,21 @@ const (
)

// IdentityType represents different types of identities.
// +kubebuilder:validation:Enum=ServicePrincipal;ManualServicePrincipal;UserAssignedMSI
// +kubebuilder:validation:Enum=ServicePrincipal;ManualServicePrincipal;ServicePrincipalCertificate
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like a breaking change, is that intended/allowed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It technically breaking but in practice, no one should be broken by the change. Right now if you set it to UserAssignedMSI it won't work and just throw a validation error in the controller. It has never been supported and there is an issue to add support for it (planning on working on that next). I could leave it as-is but IMO this is an improvement as it's better to fail the user right away if they try to use an unsupported value than to let them create the resource and later fail in the controller.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense!

type IdentityType string

const (
// UserAssignedMSI represents a user-assigned identity.
UserAssignedMSI IdentityType = "UserAssignedMSI"

// ServicePrincipal represents a service principal.
// ServicePrincipal represents a service principal using a client password as secret.
ServicePrincipal IdentityType = "ServicePrincipal"

// ManualServicePrincipal represents a manual service principal.
ManualServicePrincipal IdentityType = "ManualServicePrincipal"

// ServicePrincipalCertificate represents a service principal using a certificate as secret.
ServicePrincipalCertificate IdentityType = "ServicePrincipalCertificate"
)

// OSDisk defines the operating system disk for a VM.
Expand Down
19 changes: 7 additions & 12 deletions azure/scope/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"reflect"

aadpodid "github.com/Azure/aad-pod-identity/pkg/apis/aadpodidentity"
aadpodv1 "github.com/Azure/aad-pod-identity/pkg/apis/aadpodidentity/v1"
"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/adal"
Expand Down Expand Up @@ -87,10 +88,6 @@ func NewAzureClusterCredentialsProvider(ctx context.Context, kubeClient client.C
return nil, errors.Errorf("failed to retrieve AzureClusterIdentity external object %q/%q: %v", key.Namespace, key.Name, err)
}

if identity.Spec.Type != infrav1.ServicePrincipal {
return nil, errors.New("AzureClusterIdentity is not of type Service Principal")
}

return &AzureClusterCredentialsProvider{
AzureCredentialsProvider{
Client: kubeClient,
Expand Down Expand Up @@ -123,10 +120,6 @@ func NewManagedControlPlaneCredentialsProvider(ctx context.Context, kubeClient c
return nil, errors.Errorf("failed to retrieve AzureClusterIdentity external object %q/%q: %v", key.Namespace, key.Name, err)
}

if identity.Spec.Type != infrav1.ServicePrincipal {
return nil, errors.New("AzureClusterIdentity is not of type Service Principal")
}

return &ManagedControlPlaneCredentialsProvider{
AzureCredentialsProvider{
Client: kubeClient,
Expand All @@ -145,7 +138,7 @@ func (p *ManagedControlPlaneCredentialsProvider) GetAuthorizer(ctx context.Conte
func (p *AzureCredentialsProvider) GetAuthorizer(ctx context.Context, resourceManagerEndpoint, activeDirectoryEndpoint string, clusterMeta metav1.ObjectMeta) (autorest.Authorizer, error) {
var spt *adal.ServicePrincipalToken
switch p.Identity.Spec.Type {
case infrav1.ServicePrincipal:
case infrav1.ServicePrincipal, infrav1.ServicePrincipalCertificate:
if err := createAzureIdentityWithBindings(ctx, p.Identity, resourceManagerEndpoint, activeDirectoryEndpoint, clusterMeta, p.Client); err != nil {
return nil, err
}
Expand Down Expand Up @@ -283,13 +276,15 @@ func createAzureIdentityWithBindings(ctx context.Context, azureIdentity *infrav1

func getAzureIdentityType(identity *infrav1.AzureClusterIdentity) (aadpodv1.IdentityType, error) {
switch identity.Spec.Type {
case infrav1.ServicePrincipal:
return aadpodv1.ServicePrincipal, nil
case infrav1.UserAssignedMSI:
return aadpodv1.UserAssignedMSI, nil
case infrav1.ServicePrincipal:
return aadpodv1.ServicePrincipal, nil
case infrav1.ServicePrincipalCertificate:
return aadpodv1.IdentityType(aadpodid.ServicePrincipalCertificate), nil
}

return 0, errors.New("AzureIdentity does not have a vaild type")
return -1, errors.New("AzureIdentity does not have a valid type")
}

// IsClusterNamespaceAllowed indicates if the cluster namespace is allowed.
Expand Down
33 changes: 32 additions & 1 deletion azure/scope/identity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"testing"

aadpodid "github.com/Azure/aad-pod-identity/pkg/apis/aadpodidentity"
aadpodv1 "github.com/Azure/aad-pod-identity/pkg/apis/aadpodidentity/v1"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -212,6 +213,36 @@ func TestCreateAzureIdentityWithBindings(t *testing.T) {
Namespace: "capz-system",
},
},
{
name: "create service principal with certificate identity",
identity: &infrav1.AzureClusterIdentity{
ObjectMeta: metav1.ObjectMeta{
Name: "test-identity",
},
Spec: infrav1.AzureClusterIdentitySpec{
Type: infrav1.ServicePrincipalCertificate,
ResourceID: "my-resource-id",
ClientID: "my-client-id",
ClientSecret: corev1.SecretReference{Name: "my-client-secret"},
TenantID: "my-tenant-id",
},
},
identityType: aadpodv1.IdentityType(aadpodid.ServicePrincipalCertificate),
resourceManagerEndpoint: "public-cloud-endpoint",
activeDirectoryEndpoint: "active-directory-endpoint",
clusterMeta: metav1.ObjectMeta{
Name: "cluster-name",
Namespace: "my-namespace",
},
copiedIdentity: metav1.ObjectMeta{
Name: "cluster-name-my-namespace-test-identity",
Namespace: "capz-system",
},
binding: metav1.ObjectMeta{
Name: "cluster-name-my-namespace-test-identity-binding",
Namespace: "capz-system",
},
},
{
name: "invalid identity type",
identity: &infrav1.AzureClusterIdentity{
Expand All @@ -226,7 +257,7 @@ func TestCreateAzureIdentityWithBindings(t *testing.T) {
TenantID: "my-tenant-id",
},
},
identityType: 0,
identityType: -1,
expectedErr: true,
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,8 @@ spec:
type: object
type: object
clientID:
description: Both User Assigned MSI and SP can use this field.
description: ClientID is the service principal client ID. Both User
Assigned MSI and SP can use this field.
type: string
clientSecret:
description: ClientSecret is a secret reference which should contain
Expand All @@ -419,17 +420,19 @@ spec:
type: string
type: object
resourceID:
description: User assigned MSI resource id.
description: ResourceID is the Azure resource ID for the User Assigned
MSI resource. Not currently supported.
type: string
tenantID:
description: Service principal primary tenant id.
description: TenantID is the service principal primary tenant id.
type: string
type:
description: UserAssignedMSI or Service Principal
description: Type is the type of Azure Identity used. ServicePrincipal,
ServicePrincipalCertificate, or ManualServicePrincipal.
enum:
- ServicePrincipal
- ManualServicePrincipal
- UserAssignedMSI
- ServicePrincipalCertificate
type: string
required:
- clientID
Expand Down
49 changes: 45 additions & 4 deletions docs/book/src/topics/multitenancy.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

To enable single controller multi-tenancy, a different Identity can be added to the Azure Cluster that will be used as the Azure Identity when creating Azure resources related to that cluster.

This is achieved using the [aad-pod-identity](https://azure.github.io/aad-pod-identity) library.
This is achieved using the [aad-pod-identity](https://azure.github.io/aad-pod-identity) library.

## Service Principal Identity
## Service Principal With Client Password

Once a new SP Identity is created in Azure, the corresponding values should be used to create an `AzureClusterIdentity` resource:

Expand All @@ -22,9 +22,15 @@ spec:
allowedNamespaces:
list:
- <cluster-namespace>
```

A Kubernetes Secret should also be created to store the client password:

```bash
kubectl create secret generic "${AZURE_CLUSTER_IDENTITY_SECRET_NAME}" --from-literal=clientSecret="${AZURE_CLIENT_SECRET}"
```
The password will need to be added in a secret similar to the following example:

The resulting Secret should look similar to the following example:

```yaml
apiVersion: v1
Expand All @@ -36,7 +42,39 @@ data:
clientSecret: <client-secret-of-SP-identity>
```

OR the password can also be added as a Certificate:
## Service Principal With Certificate

Once a new SP Identity is created in Azure, the corresponding values should be used to create an `AzureClusterIdentity` resource:

```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>
clientSecret: {"name":"<secret-name-for-client-password>","namespace":"default"}
allowedNamespaces:
list:
- <cluster-namespace>
```

If needed, convert the PEM file to PKCS12 and set a password:

```bash
openssl pkcs12 -export -in fileWithCertAndPrivateKey.pem -out ad-sp-cert.pfx -passout pass:<password>
```

Create a k8s secret with the certificate and password:

```bash
kubectl create secret generic "${AZURE_CLUSTER_IDENTITY_SECRET_NAME}" --from-file=certificate=ad-sp-cert.pfx --from-literal=password=<password>
```

The resulting Secret should look similar to the following example:

```yaml
apiVersion: v1
Expand All @@ -53,6 +91,7 @@ data:

Manual Service Principal Identity is similar to [Service Principal Identity](https://capz.sigs.k8s.io/topics/multitenancy.html#service-principal-identity) except that the service principal's `clientSecret` is directly fetched from the secret containing it.
To use this type of identity, set the identity type as `ManualServicePrincipal` in `AzureClusterIdentity`. For example,

```yaml
apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: AzureClusterIdentity
Expand All @@ -68,9 +107,11 @@ spec:
list:
- <cluster-namespace>
```

The rest of the configuration is the same as that of service principal identity. This useful in scenarios where you don't want to have a dependency on [aad-pod-identity](https://azure.github.io/aad-pod-identity).

## allowedNamespaces

AllowedNamespaces is used to identify the namespaces the clusters are allowed to use the identity from. Namespaces can be selected either using an array of namespaces or with label selector.
An empty allowedNamespaces object indicates that AzureClusters can use this identity from any namespace.
If this object is nil, no namespaces will be allowed (default behaviour, if this field is not provided)
Expand Down