Skip to content

Commit

Permalink
Add DisableExtensionOperations field
Browse files Browse the repository at this point in the history
This option when set to true ensures no that no VMExtension are configured on the
machine by preventing users to add values to VMExtensions field in machine template,
Disabling the bootstrapping VMExtension and setting AllowExtensionOperations false
in the instance osProfile.
  • Loading branch information
RadekManak committed May 14, 2024
1 parent 43e72a9 commit 087217a
Show file tree
Hide file tree
Showing 12 changed files with 172 additions and 3 deletions.
6 changes: 6 additions & 0 deletions api/v1beta1/azuremachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,12 @@ type AzureMachineSpec struct {
// +optional
DNSServers []string `json:"dnsServers,omitempty"`

// DisableExtensionOperations specifies whether extension operations should be disabled on the virtual machine.
// Use this setting only if VMExtensions are not supported by your image, as it disables CAPZ bootstrapping extension used for detecting Kubernetes bootstrap failure.
// This may only be set to True when no extensions are configured on the virtual machine.
// +optional
DisableExtensionOperations *bool `json:"disableExtensionOperations,omitempty"`

// VMExtensions specifies a list of extensions to be added to the virtual machine.
// +optional
VMExtensions []VMExtension `json:"vmExtensions,omitempty"`
Expand Down
16 changes: 16 additions & 0 deletions api/v1beta1/azuremachine_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/google/uuid"
"golang.org/x/crypto/ssh"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/utils/ptr"
azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure"
)

Expand Down Expand Up @@ -71,6 +72,10 @@ func ValidateAzureMachineSpec(spec AzureMachineSpec) field.ErrorList {
allErrs = append(allErrs, errs...)
}

if errs := ValidateVMExtensions(spec.DisableExtensionOperations, spec.VMExtensions, field.NewPath("vmExtensions")); len(errs) > 0 {
allErrs = append(allErrs, errs...)
}

return allErrs
}

Expand Down Expand Up @@ -471,3 +476,14 @@ func ValidateCapacityReservationGroupID(capacityReservationGroupID *string, fldP

return allErrs
}

// ValidateVMExtensions validates the VMExtensions spec.
func ValidateVMExtensions(disableExtensionOperations *bool, vmExtensions []VMExtension, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if ptr.Deref(disableExtensionOperations, false) && len(vmExtensions) > 0 {
allErrs = append(allErrs, field.Forbidden(field.NewPath("AzureMachineTemplate", "spec", "template", "spec", "VMExtensions"), "VMExtensions must be empty when DisableExtensionOperations is true"))
}

return allErrs
}
7 changes: 7 additions & 0 deletions api/v1beta1/azuremachine_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,13 @@ func (mw *azureMachineWebhook) ValidateUpdate(ctx context.Context, oldObj, newOb
allErrs = append(allErrs, err)
}

if err := webhookutils.ValidateImmutable(
field.NewPath("spec", "disableExtensionOperations"),
old.Spec.DisableExtensionOperations,
m.Spec.DisableExtensionOperations); err != nil {
allErrs = append(allErrs, err)
}

if len(allErrs) == 0 {
return nil, nil
}
Expand Down
63 changes: 63 additions & 0 deletions api/v1beta1/azuremachine_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,16 @@ func TestAzureMachine_ValidateCreate(t *testing.T) {
machine: createMachineWithCapacityReservaionGroupID("invalid-capacity-group-id"),
wantErr: true,
},
{
name: "azuremachine with DisableExtensionOperations true and without VMExtensions",
machine: createMachineWithDisableExtenionOperations(),
wantErr: false,
},
{
name: "azuremachine with DisableExtensionOperations true and with VMExtension",
machine: createMachineWithDisableExtenionOperationsAndHasExtension(),
wantErr: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
Expand Down Expand Up @@ -778,6 +788,34 @@ func TestAzureMachine_ValidateUpdate(t *testing.T) {
},
wantErr: true,
},
{
name: "invalidTest: azuremachine.spec.disableExtensionOperations is immutable",
oldMachine: &AzureMachine{
Spec: AzureMachineSpec{
DisableExtensionOperations: ptr.To(true),
},
},
newMachine: &AzureMachine{
Spec: AzureMachineSpec{
DisableExtensionOperations: ptr.To(false),
},
},
wantErr: true,
},
{
name: "validTest: azuremachine.spec.disableExtensionOperations is immutable",
oldMachine: &AzureMachine{
Spec: AzureMachineSpec{
DisableExtensionOperations: ptr.To(true),
},
},
newMachine: &AzureMachine{
Spec: AzureMachineSpec{
DisableExtensionOperations: ptr.To(true),
},
},
wantErr: false,
},
{
name: "validTest: azuremachine.spec.networkInterfaces is immutable",
oldMachine: &AzureMachine{
Expand Down Expand Up @@ -1154,3 +1192,28 @@ func createMachineWithCapacityReservaionGroupID(capacityReservationGroupID strin
},
}
}

func createMachineWithDisableExtenionOperationsAndHasExtension() *AzureMachine {
return &AzureMachine{
Spec: AzureMachineSpec{
SSHPublicKey: validSSHPublicKey,
OSDisk: validOSDisk,
DisableExtensionOperations: ptr.To(true),
VMExtensions: []VMExtension{{
Name: "test-extension",
Publisher: "test-publiher",
Version: "v0.0.1-test",
}},
},
}
}

func createMachineWithDisableExtenionOperations() *AzureMachine {
return &AzureMachine{
Spec: AzureMachineSpec{
SSHPublicKey: validSSHPublicKey,
OSDisk: validOSDisk,
DisableExtensionOperations: ptr.To(true),
},
}
}
5 changes: 5 additions & 0 deletions api/v1beta1/azuremachinetemplate_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/utils/ptr"
"sigs.k8s.io/cluster-api/util/topology"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
Expand Down Expand Up @@ -85,6 +86,10 @@ func (r *AzureMachineTemplate) ValidateCreate(ctx context.Context, obj runtime.O
}
}

if ptr.Deref(r.Spec.Template.Spec.DisableExtensionOperations, false) && len(r.Spec.Template.Spec.VMExtensions) > 0 {
allErrs = append(allErrs, field.Forbidden(field.NewPath("AzureMachineTemplate", "spec", "template", "spec", "VMExtensions"), "VMExtensions must be empty when DisableExtensionOperations is true"))
}

if len(allErrs) == 0 {
return nil, nil
}
Expand Down
10 changes: 10 additions & 0 deletions api/v1beta1/azuremachinetemplate_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,16 @@ func TestAzureMachineTemplate_ValidateCreate(t *testing.T) {
machineTemplate: createAzureMachineTemplateFromMachine(createMachineWithRoleAssignmentName()),
wantErr: true,
},
{
name: "azuremachinetemplate with DisableExtensionOperations true and without VMExtensions",
machineTemplate: createAzureMachineTemplateFromMachine(createMachineWithDisableExtenionOperations()),
wantErr: false,
},
{
name: "azuremachinetempalte with DisableExtensionOperations true and with VMExtension",
machineTemplate: createAzureMachineTemplateFromMachine(createMachineWithDisableExtenionOperationsAndHasExtension()),
wantErr: true,
},
{
name: "azuremachinetemplate without RoleAssignmentName",
machineTemplate: createAzureMachineTemplateFromMachine(createMachineWithoutRoleAssignmentName()),
Expand Down
5 changes: 5 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go

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

5 changes: 5 additions & 0 deletions azure/scope/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ func (m *MachineScope) VMSpec() azure.ResourceSpecGetter {
SpotVMOptions: m.AzureMachine.Spec.SpotVMOptions,
SecurityProfile: m.AzureMachine.Spec.SecurityProfile,
DiagnosticsProfile: m.AzureMachine.Spec.Diagnostics,
DisableExtensionOperations: ptr.Deref(m.AzureMachine.Spec.DisableExtensionOperations, false),

Check warning on line 181 in azure/scope/machine.go

View check run for this annotation

Codecov / codecov/patch

azure/scope/machine.go#L181

Added line #L181 was not covered by tests
AdditionalTags: m.AdditionalTags(),
AdditionalCapabilities: m.AzureMachine.Spec.AdditionalCapabilities,
CapacityReservationGroupID: m.GetCapacityReservationGroupID(),
Expand Down Expand Up @@ -374,6 +375,10 @@ func (m *MachineScope) HasSystemAssignedIdentity() bool {

// VMExtensionSpecs returns the VM extension specs.
func (m *MachineScope) VMExtensionSpecs() []azure.ResourceSpecGetter {
if ptr.Deref(m.AzureMachine.Spec.DisableExtensionOperations, false) {
return []azure.ResourceSpecGetter{}
}

var extensionSpecs = []azure.ResourceSpecGetter{}
for _, extension := range m.AzureMachine.Spec.VMExtensions {
extensionSpecs = append(extensionSpecs, &vmextensions.VMExtensionSpec{
Expand Down
38 changes: 38 additions & 0 deletions azure/scope/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,44 @@ func TestMachineScope_VMExtensionSpecs(t *testing.T) {
},
},
},
{
name: "If OS type is Linux and cloud is AzurePublicCloud and DisableExtensionOperations is true, it returns empty",
machineScope: MachineScope{
Machine: &clusterv1.Machine{},
AzureMachine: &infrav1.AzureMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "machine-name",
},
Spec: infrav1.AzureMachineSpec{
DisableExtensionOperations: ptr.To(true),
OSDisk: infrav1.OSDisk{
OSType: "Linux",
},
},
},
ClusterScoper: &ClusterScope{
AzureClients: AzureClients{
EnvironmentSettings: auth.EnvironmentSettings{
Environment: azureautorest.Environment{
Name: azureautorest.PublicCloud.Name,
},
},
},
AzureCluster: &infrav1.AzureCluster{
Spec: infrav1.AzureClusterSpec{
ResourceGroup: "my-rg",
AzureClusterClassSpec: infrav1.AzureClusterClassSpec{
Location: "westus",
},
},
},
},
cache: &MachineCache{
VMSKU: resourceskus.SKU{},
},
},
want: []azure.ResourceSpecGetter{},
},
{
name: "If OS type is Linux and cloud is not AzurePublicCloud, it returns empty",
machineScope: MachineScope{
Expand Down
8 changes: 5 additions & 3 deletions azure/services/virtualmachines/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type VMSpec struct {
AdditionalTags infrav1.Tags
AdditionalCapabilities *infrav1.AdditionalCapabilities
DiagnosticsProfile *infrav1.Diagnostics
DisableExtensionOperations bool
CapacityReservationGroupID string
SKU resourceskus.SKU
Image *infrav1.Image
Expand Down Expand Up @@ -263,9 +264,10 @@ func (s *VMSpec) generateOSProfile() (*armcompute.OSProfile, error) {
}

osProfile := &armcompute.OSProfile{
ComputerName: ptr.To(s.Name),
AdminUsername: ptr.To(azure.DefaultUserName),
CustomData: ptr.To(s.BootstrapData),
ComputerName: ptr.To(s.Name),
AdminUsername: ptr.To(azure.DefaultUserName),
CustomData: ptr.To(s.BootstrapData),
AllowExtensionOperations: ptr.To(!s.DisableExtensionOperations),
}

switch s.OSDisk.OSType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,12 @@ spec:
- storageAccountType
type: object
type: object
disableExtensionOperations:
description: |-
DisableExtensionOperations specifies whether extension operations should be disabled on the virtual machine.
Use this setting only if VMExtensions are not supported by your image, as it disables CAPZ bootstrapping extension used for detecting Kubernetes bootstrap failure.
This may only be set to True when no extensions are configured on the virtual machine.
type: boolean
dnsServers:
description: DNSServers adds a list of DNS Server IP addresses to
the VM NICs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,12 @@ spec:
- storageAccountType
type: object
type: object
disableExtensionOperations:
description: |-
DisableExtensionOperations specifies whether extension operations should be disabled on the virtual machine.
Use this setting only if VMExtensions are not supported by your image, as it disables CAPZ bootstrapping extension used for detecting Kubernetes bootstrap failure.
This may only be set to True when no extensions are configured on the virtual machine.
type: boolean
dnsServers:
description: DNSServers adds a list of DNS Server IP addresses
to the VM NICs.
Expand Down

0 comments on commit 087217a

Please sign in to comment.