From cc0b7c5f5160a16f3a491c413774a08ffbc47039 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 4a4b444133..e49583fe14 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/openshift/api/features" @@ -3399,6 +3400,7 @@ func TestDefaultAzureProviderSpec(t *testing.T) { testCase string providerSpec *machinev1beta1.AzureMachineProviderSpec modifyDefault func(*machinev1beta1.AzureMachineProviderSpec) + platformStatus *osconfigv1.PlatformStatus expectedError string expectedOk bool expectedWarnings []string @@ -3448,6 +3450,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{ @@ -3494,10 +3519,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 46458dc16aa7001267b7eef3dbd3f437edf990a7 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 4b04ed2834..1cbaeece77 100644 --- a/pkg/webhooks/machine_webhook.go +++ b/pkg/webhooks/machine_webhook.go @@ -60,7 +60,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 @@ -70,7 +70,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) @@ -920,7 +920,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 e49583fe14..ba2a969fe1 100644 --- a/pkg/webhooks/machine_webhook_test.go +++ b/pkg/webhooks/machine_webhook_test.go @@ -1495,7 +1495,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), @@ -3532,7 +3532,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 2336429eca..a6b30a606a 100644 --- a/pkg/webhooks/machineset_webhook_test.go +++ b/pkg/webhooks/machineset_webhook_test.go @@ -603,7 +603,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 f203cde9ed198c0eeb2352d44889a000b4fc8b5e 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 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/webhooks/machine_webhook.go b/pkg/webhooks/machine_webhook.go index 1cbaeece77..ca6dbe9ce5 100644 --- a/pkg/webhooks/machine_webhook.go +++ b/pkg/webhooks/machine_webhook.go @@ -920,7 +920,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 config.platformStatus.Azure != nil && config.platformStatus.Azure.ResourceGroupName != "" { + rg = config.platformStatus.Azure.ResourceGroupName + } + providerSpec.Image.ResourceID = defaultAzureImageResourceID(config.clusterID, rg) } if providerSpec.UserDataSecret == nil {