Skip to content

Commit

Permalink
Allow for configurable scope in system assigned identities
Browse files Browse the repository at this point in the history
  • Loading branch information
willie-yao committed Feb 2, 2023
1 parent 8c66502 commit 1a26e66
Show file tree
Hide file tree
Showing 35 changed files with 980 additions and 160 deletions.
4 changes: 4 additions & 0 deletions api/v1alpha3/azuremachine_conversion.go
Expand Up @@ -78,6 +78,10 @@ func (src *AzureMachine) ConvertTo(dstRaw conversion.Hub) error {
dst.Spec.NetworkInterfaces = restored.Spec.NetworkInterfaces
}

if restored.Spec.SystemAssignedIdentityRole != nil {
dst.Spec.SystemAssignedIdentityRole = restored.Spec.SystemAssignedIdentityRole
}

//nolint:staticcheck // SubnetName is now deprecated, but the v1beta1 defaulting webhook will migrate it to the networkInterfaces field
dst.Spec.SubnetName = restored.Spec.SubnetName

Expand Down
4 changes: 4 additions & 0 deletions api/v1alpha3/azuremachinetemplate_conversion.go
Expand Up @@ -82,6 +82,10 @@ func (src *AzureMachineTemplate) ConvertTo(dstRaw conversion.Hub) error {
dst.Spec.Template.Spec.NetworkInterfaces = restored.Spec.Template.Spec.NetworkInterfaces
}

if restored.Spec.Template.Spec.SystemAssignedIdentityRole != nil {
dst.Spec.Template.Spec.SystemAssignedIdentityRole = restored.Spec.Template.Spec.SystemAssignedIdentityRole
}

return nil
}

Expand Down
1 change: 1 addition & 0 deletions api/v1alpha3/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions api/v1alpha4/azuremachine_conversion.go
Expand Up @@ -64,6 +64,10 @@ func (src *AzureMachine) ConvertTo(dstRaw conversion.Hub) error {
dst.Spec.NetworkInterfaces = restored.Spec.NetworkInterfaces
}

if restored.Spec.SystemAssignedIdentityRole != nil {
dst.Spec.SystemAssignedIdentityRole = restored.Spec.SystemAssignedIdentityRole
}

return nil
}

Expand Down
4 changes: 4 additions & 0 deletions api/v1alpha4/azuremachinetemplate_conversion.go
Expand Up @@ -66,6 +66,10 @@ func (src *AzureMachineTemplate) ConvertTo(dstRaw conversion.Hub) error {
dst.Spec.Template.Spec.NetworkInterfaces = restored.Spec.Template.Spec.NetworkInterfaces
}

if restored.Spec.Template.Spec.SystemAssignedIdentityRole != nil {
dst.Spec.Template.Spec.SystemAssignedIdentityRole = restored.Spec.Template.Spec.SystemAssignedIdentityRole
}

return nil
}

Expand Down
1 change: 1 addition & 0 deletions api/v1alpha4/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

94 changes: 81 additions & 13 deletions api/v1beta1/azuremachine_default.go
Expand Up @@ -17,15 +17,24 @@ limitations under the License.
package v1beta1

import (
"context"
"encoding/base64"
"fmt"
"time"

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
"github.com/pkg/errors"
"golang.org/x/crypto/ssh"
"k8s.io/apimachinery/pkg/util/uuid"
utilSSH "sigs.k8s.io/cluster-api-provider-azure/util/ssh"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// ContributorRoleID is the ID of the built-in "Contributor" role.
const ContributorRoleID = "b24988ac-6180-42a0-ab88-20f7382dd24c"

// SetDefaultSSHPublicKey sets the default SSHPublicKey for an AzureMachine.
func (s *AzureMachineSpec) SetDefaultSSHPublicKey() error {
if sshKeyData := s.SSHPublicKey; sshKeyData == "" {
Expand Down Expand Up @@ -79,10 +88,32 @@ func (s *AzureMachineSpec) SetDataDisksDefaults() {
}

// SetIdentityDefaults sets the defaults for VM Identity.
func (s *AzureMachineSpec) SetIdentityDefaults() {
func (s *AzureMachineSpec) SetIdentityDefaults(subscriptionID string) {
// Ensure the deprecated fields and new fields are not populated simultaneously
if s.RoleAssignmentName != "" && s.SystemAssignedIdentityRole != nil && s.SystemAssignedIdentityRole.Name != "" {
// Both the deprecated and the new fields are both set, return without changes
// and reject the request in the validating webhook which runs later.
return
}
if s.Identity == VMIdentitySystemAssigned {
if s.RoleAssignmentName == "" {
s.RoleAssignmentName = string(uuid.NewUUID())
if s.SystemAssignedIdentityRole == nil {
s.SystemAssignedIdentityRole = &SystemAssignedIdentityRole{}
}
if s.RoleAssignmentName != "" {
// Move the existing value from the deprecated RoleAssignmentName field.
s.SystemAssignedIdentityRole.Name = s.RoleAssignmentName
s.RoleAssignmentName = ""
} else if s.SystemAssignedIdentityRole.Name == "" {
// Default role name to the role assignment name.
s.SystemAssignedIdentityRole.Name = string(uuid.NewUUID())
}
if s.SystemAssignedIdentityRole.Scope == "" && subscriptionID != "" {
// Default scope to the subscription.
s.SystemAssignedIdentityRole.Scope = fmt.Sprintf("/subscriptions/%s/", subscriptionID)
}
if s.SystemAssignedIdentityRole.DefinitionID == "" && subscriptionID != "" {
// Default role definition ID to Contributor role.
s.SystemAssignedIdentityRole.DefinitionID = fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Authorization/roleDefinitions/%s", subscriptionID, ContributorRoleID)
}
}
}
Expand Down Expand Up @@ -143,15 +174,52 @@ func (s *AzureMachineSpec) SetNetworkInterfacesDefaults() {
}
}

// GetSubscriptionID returns the subscription ID for the given cluster name and namespace.
func GetSubscriptionID(cli client.Client, clusterName string, namespace string, maxAttempts int) (string, error) {
ctx := context.Background()

ownerCluster := &AzureCluster{}
key := client.ObjectKey{
Namespace: namespace,
Name: clusterName,
}

for i := 1; ; i++ {
err := cli.Get(ctx, key, ownerCluster)
if err != nil {
if i > maxAttempts {
return "", errors.Wrapf(err, "failed to find owner cluster for %s/%s", namespace, clusterName)
}
time.Sleep(1 * time.Second)
continue
}
break
}

return ownerCluster.Spec.SubscriptionID, nil
}

// SetDefaults sets to the defaults for the AzureMachineSpec.
func (s *AzureMachineSpec) SetDefaults() {
if err := s.SetDefaultSSHPublicKey(); err != nil {
ctrl.Log.WithName("SetDefault").Error(err, "SetDefaultSshPublicKey failed")
}
s.SetDefaultCachingType()
s.SetDataDisksDefaults()
s.SetIdentityDefaults()
s.SetSpotEvictionPolicyDefaults()
s.SetDiagnosticsDefaults()
s.SetNetworkInterfacesDefaults()
func (m *AzureMachine) SetDefaults(client client.Client) {
if err := m.Spec.SetDefaultSSHPublicKey(); err != nil {
ctrl.Log.WithName("SetDefault").Error(err, "failed to set default SSH public key")
}

// Fetch the Cluster.
clusterName, ok := m.Labels[clusterv1.ClusterLabelName]
if !ok {
ctrl.Log.WithName("SetDefault").Error(errors.Errorf("failed to fetch owner ClusterName for AzureMachine %s/%s", m.Namespace, m.Name), "failed to fetch ClusterName")
}

subscriptionID, err := GetSubscriptionID(client, clusterName, m.Namespace, 5)
if err != nil {
ctrl.Log.WithName("SetDefault").Error(err, "failed to fetch subscription ID for AzureMachine %s/%s", m.Namespace, m.Name)
}

m.Spec.SetDefaultCachingType()
m.Spec.SetDataDisksDefaults()
m.Spec.SetIdentityDefaults(subscriptionID)
m.Spec.SetSpotEvictionPolicyDefaults()
m.Spec.SetDiagnosticsDefaults()
m.Spec.SetNetworkInterfacesDefaults()
}
52 changes: 37 additions & 15 deletions api/v1beta1/azuremachine_default_test.go
Expand Up @@ -18,6 +18,7 @@ package v1beta1

import (
"encoding/json"
"fmt"
"reflect"
"testing"

Expand Down Expand Up @@ -53,29 +54,50 @@ func TestAzureMachineSpec_SetIdentityDefaults(t *testing.T) {
machine *AzureMachine
}

fakeSubscriptionID := uuid.New().String()
fakeClusterName := "testcluster"
fakeRoleDefinitionID := "testroledefinitionid"
fakeScope := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s", fakeSubscriptionID, fakeClusterName)
existingRoleAssignmentName := "42862306-e485-4319-9bf0-35dbc6f6fe9c"
roleAssignmentExistTest := test{machine: &AzureMachine{Spec: AzureMachineSpec{
Identity: VMIdentitySystemAssigned,
RoleAssignmentName: existingRoleAssignmentName,
}}}
roleAssignmentEmptyTest := test{machine: &AzureMachine{Spec: AzureMachineSpec{
Identity: VMIdentitySystemAssigned,
RoleAssignmentName: "",
Identity: VMIdentitySystemAssigned,
SystemAssignedIdentityRole: &SystemAssignedIdentityRole{
Name: existingRoleAssignmentName,
},
}}}
notSystemAssignedTest := test{machine: &AzureMachine{Spec: AzureMachineSpec{
Identity: VMIdentityUserAssigned,
Identity: VMIdentityUserAssigned,
SystemAssignedIdentityRole: &SystemAssignedIdentityRole{},
}}}
systemAssignedIdentityRoleExistTest := test{machine: &AzureMachine{Spec: AzureMachineSpec{
Identity: VMIdentitySystemAssigned,
SystemAssignedIdentityRole: &SystemAssignedIdentityRole{
Scope: fakeScope,
DefinitionID: fakeRoleDefinitionID,
},
}}}

roleAssignmentExistTest.machine.Spec.SetIdentityDefaults()
g.Expect(roleAssignmentExistTest.machine.Spec.RoleAssignmentName).To(Equal(existingRoleAssignmentName))
emptyTest := test{machine: &AzureMachine{Spec: AzureMachineSpec{
Identity: VMIdentitySystemAssigned,
SystemAssignedIdentityRole: &SystemAssignedIdentityRole{},
}}}

roleAssignmentEmptyTest.machine.Spec.SetIdentityDefaults()
g.Expect(roleAssignmentEmptyTest.machine.Spec.RoleAssignmentName).To(Not(BeEmpty()))
_, err := uuid.Parse(roleAssignmentEmptyTest.machine.Spec.RoleAssignmentName)
g.Expect(err).To(Not(HaveOccurred()))
roleAssignmentExistTest.machine.Spec.SetIdentityDefaults(fakeSubscriptionID)
g.Expect(roleAssignmentExistTest.machine.Spec.SystemAssignedIdentityRole.Name).To(Equal(existingRoleAssignmentName))

notSystemAssignedTest.machine.Spec.SetIdentityDefaults(fakeSubscriptionID)
g.Expect(notSystemAssignedTest.machine.Spec.SystemAssignedIdentityRole.Name).To(BeEmpty())

notSystemAssignedTest.machine.Spec.SetIdentityDefaults()
g.Expect(notSystemAssignedTest.machine.Spec.RoleAssignmentName).To(BeEmpty())
systemAssignedIdentityRoleExistTest.machine.Spec.SetIdentityDefaults(fakeSubscriptionID)
g.Expect(systemAssignedIdentityRoleExistTest.machine.Spec.SystemAssignedIdentityRole.Scope).To(Equal(fakeScope))
g.Expect(systemAssignedIdentityRoleExistTest.machine.Spec.SystemAssignedIdentityRole.DefinitionID).To(Equal(fakeRoleDefinitionID))

emptyTest.machine.Spec.SetIdentityDefaults(fakeSubscriptionID)
g.Expect(emptyTest.machine.Spec.SystemAssignedIdentityRole.Name).To(Not(BeEmpty()))
_, err := uuid.Parse(emptyTest.machine.Spec.SystemAssignedIdentityRole.Name)
g.Expect(err).To(Not(HaveOccurred()))
g.Expect(emptyTest.machine.Spec.SystemAssignedIdentityRole.Scope).To(Equal(fmt.Sprintf("/subscriptions/%s/", fakeSubscriptionID)))
g.Expect(emptyTest.machine.Spec.SystemAssignedIdentityRole.DefinitionID).To(Equal(fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Authorization/roleDefinitions/%s", fakeSubscriptionID, ContributorRoleID)))
}

func TestAzureMachineSpec_SetDataDisksDefaults(t *testing.T) {
Expand Down
25 changes: 23 additions & 2 deletions api/v1beta1/azuremachine_types.go
Expand Up @@ -66,8 +66,11 @@ type AzureMachineSpec struct {
// +optional
UserAssignedIdentities []UserAssignedIdentity `json:"userAssignedIdentities,omitempty"`

// RoleAssignmentName is the name of the role assignment to create for a system assigned identity. It can be any valid GUID.
// If not specified, a random GUID will be generated.
// SystemAssignedIdentityRole defines the role and scope to assign to the system assigned identity.
// +optional
SystemAssignedIdentityRole *SystemAssignedIdentityRole `json:"systemAssignedIdentityRole,omitempty"`

// Deprecated: RoleAssignmentName should be set in the systemAssignedIdentityRole field.
// +optional
RoleAssignmentName string `json:"roleAssignmentName,omitempty"`

Expand Down Expand Up @@ -149,6 +152,24 @@ type SpotVMOptions struct {
EvictionPolicy *SpotEvictionPolicy `json:"evictionPolicy,omitempty"`
}

// SystemAssignedIdentityRole defines the role and scope to assign to the system assigned identity.
type SystemAssignedIdentityRole struct {
// Name is the name of the role assignment to create for a system assigned identity. It can be any valid GUID.
// If not specified, a random GUID will be generated.
// +optional
Name string `json:"name,omitempty"`

// DefinitionID is the ID of the role definition to create for a system assigned identity. It can be an Azure built-in role or a custom role.
// Refer to built-in roles: https://learn.microsoft.com/en-us/azure/role-based-access-control/built-in-roles
// +optional
DefinitionID string `json:"definitionID,omitempty"`

// Scope is the scope that the role assignment or definition applies to. The scope can be any REST resource instance.
// If not specified, the scope will be the subscription.
// +optional
Scope string `json:"scope,omitempty"`
}

// AzureMachineStatus defines the observed state of AzureMachine.
type AzureMachineStatus struct {
// Ready is true when the provider resource is ready.
Expand Down
28 changes: 26 additions & 2 deletions api/v1beta1/azuremachine_validation.go
Expand Up @@ -58,6 +58,10 @@ func ValidateAzureMachineSpec(spec AzureMachineSpec) field.ErrorList {
allErrs = append(allErrs, errs...)
}

if errs := ValidateSystemAssignedIdentityRole(spec.Identity, spec.RoleAssignmentName, spec.SystemAssignedIdentityRole, field.NewPath("systemAssignedIdentityRole")); len(errs) > 0 {
allErrs = append(allErrs, errs...)
}

return allErrs
}

Expand Down Expand Up @@ -117,15 +121,35 @@ func ValidateSystemAssignedIdentity(identityType VMIdentity, oldIdentity, newIde
}

// ValidateUserAssignedIdentity validates the user-assigned identities list.
func ValidateUserAssignedIdentity(identityType VMIdentity, userAssignedIdenteties []UserAssignedIdentity, fldPath *field.Path) field.ErrorList {
func ValidateUserAssignedIdentity(identityType VMIdentity, userAssignedIdentities []UserAssignedIdentity, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if identityType == VMIdentityUserAssigned && len(userAssignedIdenteties) == 0 {
if identityType == VMIdentityUserAssigned && len(userAssignedIdentities) == 0 {
allErrs = append(allErrs, field.Required(fldPath, "must be specified for the 'UserAssigned' identity type"))
}
return allErrs
}

// ValidateSystemAssignedIdentityRole validates the system-assigned identity role.
func ValidateSystemAssignedIdentityRole(identityType VMIdentity, roleAssignmentName string, role *SystemAssignedIdentityRole, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
if roleAssignmentName != "" && role != nil && role.Name != "" {
allErrs = append(allErrs, field.Invalid(fldPath, role.Name, "cannot set both roleAssignmentName and systemAssignedIdentityRole.name"))
}
if identityType == VMIdentitySystemAssigned {
if role.DefinitionID == "" {
allErrs = append(allErrs, field.Invalid(field.NewPath("Spec", "SystemAssignedIdentityRole", "DefinitionID"), role.DefinitionID, "the roleDefinitionID field cannot be empty"))
}
if role.Scope == "" {
allErrs = append(allErrs, field.Invalid(field.NewPath("Spec", "SystemAssignedIdentityRole", "Scope"), role.Scope, "the scope field cannot be empty"))
}
}
if identityType != VMIdentitySystemAssigned && role != nil {
allErrs = append(allErrs, field.Forbidden(field.NewPath("Spec", "Role"), "systemAssignedIdentityRole can only be set when identity is set to SystemAssigned"))
}
return allErrs
}

// ValidateDataDisks validates a list of data disks.
func ValidateDataDisks(dataDisks []DataDisk, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand Down

0 comments on commit 1a26e66

Please sign in to comment.