From a3f5a933de6aaf995f50c3c3bdc91b97089e47f9 Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Thu, 12 Mar 2026 23:01:03 -0400 Subject: [PATCH 1/3] webhooks: add test for azure image with custom resource group Test that if a custom resource group is set, the default image uses that in the resource id. --- pkg/webhooks/machine_webhook_test.go | 33 ++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/pkg/webhooks/machine_webhook_test.go b/pkg/webhooks/machine_webhook_test.go index 72d17602c0..eab5a5f437 100644 --- a/pkg/webhooks/machine_webhook_test.go +++ b/pkg/webhooks/machine_webhook_test.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "reflect" + "strings" "testing" . "github.com/onsi/gomega" @@ -3080,6 +3081,7 @@ func TestDefaultAzureProviderSpec(t *testing.T) { testCase string providerSpec *machinev1beta1.AzureMachineProviderSpec modifyDefault func(*machinev1beta1.AzureMachineProviderSpec) + platformStatus *osconfigv1.PlatformStatus expectedError string expectedOk bool expectedWarnings []string @@ -3129,6 +3131,29 @@ func TestDefaultAzureProviderSpec(t *testing.T) { expectedError: "", expectedWarnings: itWarnings, }, + { + testCase: "it generates azure image ResourceID with custom resource group", + providerSpec: &machinev1beta1.AzureMachineProviderSpec{ + Image: machinev1beta1.Image{}, + }, + platformStatus: &osconfigv1.PlatformStatus{ + Type: osconfigv1.AzurePlatformType, + Azure: &osconfigv1.AzurePlatformStatus{ + ResourceGroupName: "test-rg", + }, + }, + modifyDefault: func(p *machinev1beta1.AzureMachineProviderSpec) { + // Generate expected image ID with custom resource group from infra config + galleryName := strings.Replace(clusterID, "-", "_", -1) + imageName := clusterID + p.Image = machinev1beta1.Image{ + ResourceID: fmt.Sprintf("/resourceGroups/%s/providers/Microsoft.Compute/galleries/gallery_%s/images/%s/versions/%s", "test-rg", galleryName, imageName, azureRHCOSVersion), + } + }, + expectedOk: true, + expectedError: "", + expectedWarnings: itWarnings, + }, { testCase: "does not overwrite the network resource group if it already exists", providerSpec: &machinev1beta1.AzureMachineProviderSpec{ @@ -3175,10 +3200,14 @@ func TestDefaultAzureProviderSpec(t *testing.T) { }, } - platformStatus := &osconfigv1.PlatformStatus{Type: osconfigv1.AzurePlatformType} - h := createMachineDefaulter(platformStatus, clusterID) + defaultPlatformStatus := &osconfigv1.PlatformStatus{Type: osconfigv1.AzurePlatformType} for _, tc := range testCases { t.Run(tc.testCase, func(t *testing.T) { + platformStatus := defaultPlatformStatus + if tc.platformStatus != nil { + platformStatus = tc.platformStatus + } + h := createMachineDefaulter(platformStatus, clusterID) defaultProviderSpec := &machinev1beta1.AzureMachineProviderSpec{ VMSize: defaultInstanceType, Vnet: defaultAzureVnet(clusterID), From d40a0de7c01e5d227d8eb521d4258076af5db0dc Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Thu, 12 Mar 2026 23:12:57 -0400 Subject: [PATCH 2/3] OCPBUGS-78424: use resource group when generating default Azure image Prior to this, the default always assumed the resource group was in the form infraID-rg, but that is not the case for users installing into existing resource groups. Instead, read it off the infrastructure config. --- pkg/webhooks/machine_webhook.go | 6 +++--- pkg/webhooks/machine_webhook_test.go | 4 ++-- pkg/webhooks/machineset_webhook_test.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/webhooks/machine_webhook.go b/pkg/webhooks/machine_webhook.go index ae7eaf0f9d..e426f7d746 100644 --- a/pkg/webhooks/machine_webhook.go +++ b/pkg/webhooks/machine_webhook.go @@ -55,7 +55,7 @@ var ( defaultAzureNetworkResourceGroup = func(clusterID string) string { return fmt.Sprintf("%s-rg", clusterID) } - defaultAzureImageResourceID = func(clusterID string) string { + defaultAzureImageResourceID = func(clusterID, rg string) string { // image gallery names cannot have dashes galleryName := strings.Replace(clusterID, "-", "_", -1) imageName := clusterID @@ -65,7 +65,7 @@ var ( // before that change will have a -gen2 image. imageName = fmt.Sprintf("%s-gen2", clusterID) } - return fmt.Sprintf("/resourceGroups/%s/providers/Microsoft.Compute/galleries/gallery_%s/images/%s/versions/%s", clusterID+"-rg", galleryName, imageName, azureRHCOSVersion) + return fmt.Sprintf("/resourceGroups/%s/providers/Microsoft.Compute/galleries/gallery_%s/images/%s/versions/%s", rg, galleryName, imageName, azureRHCOSVersion) } defaultAzureManagedIdentiy = func(clusterID string) string { return fmt.Sprintf("%s-identity", clusterID) @@ -857,7 +857,7 @@ func defaultAzure(m *machinev1beta1.Machine, config *admissionConfig) (bool, []s } if providerSpec.Image == (machinev1beta1.Image{}) { - providerSpec.Image.ResourceID = defaultAzureImageResourceID(config.clusterID) + providerSpec.Image.ResourceID = defaultAzureImageResourceID(config.clusterID, config.platformStatus.Azure.ResourceGroupName) } if providerSpec.UserDataSecret == nil { diff --git a/pkg/webhooks/machine_webhook_test.go b/pkg/webhooks/machine_webhook_test.go index eab5a5f437..314ac27918 100644 --- a/pkg/webhooks/machine_webhook_test.go +++ b/pkg/webhooks/machine_webhook_test.go @@ -1252,7 +1252,7 @@ func TestMachineUpdate(t *testing.T) { Subnet: defaultAzureSubnet(azureClusterID), NetworkResourceGroup: defaultAzureNetworkResourceGroup(azureClusterID), Image: machinev1beta1.Image{ - ResourceID: defaultAzureImageResourceID(azureClusterID), + ResourceID: defaultAzureImageResourceID(azureClusterID, defaultAzureResourceGroup(azureClusterID)), }, ManagedIdentity: defaultAzureManagedIdentiy(azureClusterID), ResourceGroup: defaultAzureResourceGroup(azureClusterID), @@ -3213,7 +3213,7 @@ func TestDefaultAzureProviderSpec(t *testing.T) { Vnet: defaultAzureVnet(clusterID), Subnet: defaultAzureSubnet(clusterID), Image: machinev1beta1.Image{ - ResourceID: defaultAzureImageResourceID(clusterID), + ResourceID: defaultAzureImageResourceID(clusterID, defaultAzureResourceGroup(clusterID)), }, UserDataSecret: &corev1.SecretReference{ Name: defaultUserDataSecret, diff --git a/pkg/webhooks/machineset_webhook_test.go b/pkg/webhooks/machineset_webhook_test.go index 5fdfc09eed..94bb610ecb 100644 --- a/pkg/webhooks/machineset_webhook_test.go +++ b/pkg/webhooks/machineset_webhook_test.go @@ -595,7 +595,7 @@ func TestMachineSetUpdate(t *testing.T) { Subnet: defaultAzureSubnet(azureClusterID), NetworkResourceGroup: defaultAzureNetworkResourceGroup(azureClusterID), Image: machinev1beta1.Image{ - ResourceID: defaultAzureImageResourceID(azureClusterID), + ResourceID: defaultAzureImageResourceID(azureClusterID, defaultAzureResourceGroup(azureClusterID)), }, ManagedIdentity: defaultAzureManagedIdentiy(azureClusterID), ResourceGroup: defaultAzureResourceGroup(azureClusterID), From 3196d2e59235eea6903674eaf7903f60d81bafff Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Thu, 12 Mar 2026 23:33:20 -0400 Subject: [PATCH 3/3] defensively handle empty Azure resource group for default image The resource group is required on the infrastructure resource, so it should be safe to always read it. To be completely defensive we can assume the default resource group in the impossible case that resource group is not specified. --- pkg/webhooks/machine_webhook.go | 8 ++++++-- pkg/webhooks/machineset_webhook.go | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/webhooks/machine_webhook.go b/pkg/webhooks/machine_webhook.go index e426f7d746..8cce0db7c1 100644 --- a/pkg/webhooks/machine_webhook.go +++ b/pkg/webhooks/machine_webhook.go @@ -415,7 +415,7 @@ func NewMachineDefaulter() (*admission.Webhook, error) { func createMachineDefaulter(platformStatus *osconfigv1.PlatformStatus, clusterID string) *machineDefaulterHandler { return &machineDefaulterHandler{ admissionHandler: &admissionHandler{ - admissionConfig: &admissionConfig{clusterID: clusterID}, + admissionConfig: &admissionConfig{clusterID: clusterID, platformStatus: platformStatus}, webhookOperations: getMachineDefaulterOperation(platformStatus), }, } @@ -857,7 +857,11 @@ func defaultAzure(m *machinev1beta1.Machine, config *admissionConfig) (bool, []s } if providerSpec.Image == (machinev1beta1.Image{}) { - providerSpec.Image.ResourceID = defaultAzureImageResourceID(config.clusterID, config.platformStatus.Azure.ResourceGroupName) + rg := defaultAzureResourceGroup(config.clusterID) + if ps := config.platformStatus; ps != nil && ps.Azure != nil && ps.Azure.ResourceGroupName != "" { + rg = ps.Azure.ResourceGroupName + } + providerSpec.Image.ResourceID = defaultAzureImageResourceID(config.clusterID, rg) } if providerSpec.UserDataSecret == nil { diff --git a/pkg/webhooks/machineset_webhook.go b/pkg/webhooks/machineset_webhook.go index e36ff07433..dc3ff9fe2f 100644 --- a/pkg/webhooks/machineset_webhook.go +++ b/pkg/webhooks/machineset_webhook.go @@ -76,7 +76,7 @@ func NewMachineSetDefaulter() (*admission.Webhook, error) { func createMachineSetDefaulter(platformStatus *osconfigv1.PlatformStatus, clusterID string) *admission.Webhook { return admission.WithCustomDefaulter(scheme.Scheme, &machinev1beta1.MachineSet{}, &machineSetDefaulterHandler{ admissionHandler: &admissionHandler{ - admissionConfig: &admissionConfig{clusterID: clusterID}, + admissionConfig: &admissionConfig{clusterID: clusterID, platformStatus: platformStatus}, webhookOperations: getMachineDefaulterOperation(platformStatus), }, })