diff --git a/cmd/cluster/azure/destroy.go b/cmd/cluster/azure/destroy.go index 38051e95e86d..baf63d29acb4 100644 --- a/cmd/cluster/azure/destroy.go +++ b/cmd/cluster/azure/destroy.go @@ -42,8 +42,10 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { cmd.Flags().StringVar(&opts.AzurePlatform.Location, "location", opts.AzurePlatform.Location, "Location for the cluster") cmd.Flags().StringVar(&opts.AzurePlatform.ResourceGroupName, "resource-group-name", opts.AzurePlatform.ResourceGroupName, "The name of the resource group containing the HostedCluster infrastructure resources that need to be destroyed.") cmd.Flags().BoolVar(&opts.AzurePlatform.PreserveResourceGroup, "preserve-resource-group", opts.AzurePlatform.PreserveResourceGroup, "When true, the managed/main resource group will not be deleted during cluster destroy. Only cluster-specific resources within the resource group will be cleaned up.") + cmd.Flags().StringVar(&opts.AzurePlatform.DNSZoneRGName, "dns-zone-rg-name", opts.AzurePlatform.DNSZoneRGName, util.DNSZoneRGNameDestroyDescription) _ = cmd.MarkFlagRequired("azure-creds") + _ = cmd.MarkFlagRequired("dns-zone-rg-name") logger := log.Log cmd.Run = func(cmd *cobra.Command, args []string) { @@ -148,6 +150,24 @@ func DestroyCluster(ctx context.Context, o *core.DestroyOptions) error { } func destroyPlatformSpecifics(ctx context.Context, o *core.DestroyOptions) error { + // Clean up role assignments before destroying infrastructure to avoid orphans. + // Match the create path resource-group names: {name}-nsg and {name}-vnet. + subscriptionID, azureCreds, err := util.SetupAzureCredentials(o.Log, nil, o.AzurePlatform.CredentialsFile) + if err != nil { + return fmt.Errorf("failed to setup Azure credentials: %w", err) + } + + nsgRG := o.Name + "-nsg" + vnetRG := o.Name + "-vnet" + + rbacManager := azureinfra.NewRBACManager(subscriptionID, azureCreds) + // assignCustomHCPRoles=false is safe: GetServicePrincipalScopes only uses the flag to select + // the role definition ID, not to modify the scopes list. Cleanup derives role assignment names + // from infraID + component + scope, so the role ID is irrelevant. + if err := rbacManager.CleanupRoleAssignments(ctx, o.Log, o.InfraID, o.AzurePlatform.ResourceGroupName, nsgRG, vnetRG, o.AzurePlatform.DNSZoneRGName, false); err != nil { + o.Log.Error(err, "Failed to clean up some role assignments, continuing with infrastructure deletion") + } + destroyInfraOptions := &azureinfra.DestroyInfraOptions{ Name: o.Name, Location: o.AzurePlatform.Location, diff --git a/cmd/cluster/core/destroy.go b/cmd/cluster/core/destroy.go index eb1a282320b3..49020dd0e806 100644 --- a/cmd/cluster/core/destroy.go +++ b/cmd/cluster/core/destroy.go @@ -64,6 +64,7 @@ type AzurePlatformDestroyOptions struct { ResourceGroupName string PreserveResourceGroup bool Cloud string + DNSZoneRGName string } type PowerVSPlatformDestroyOptions struct { diff --git a/cmd/infra/azure/destroy_iam.go b/cmd/infra/azure/destroy_iam.go index 0ac3b3f1b295..259054da9082 100644 --- a/cmd/infra/azure/destroy_iam.go +++ b/cmd/infra/azure/destroy_iam.go @@ -32,12 +32,14 @@ func NewDestroyIAMCommand() *cobra.Command { cmd.Flags().StringVar(&opts.Cloud, "cloud", opts.Cloud, util.CloudDescription) cmd.Flags().StringVar(&opts.Name, "name", opts.Name, util.NameDescription) cmd.Flags().StringVar(&opts.InfraID, "infra-id", opts.InfraID, util.InfraIDDescription) + cmd.Flags().StringVar(&opts.DNSZoneRG, "dns-zone-rg-name", opts.DNSZoneRG, util.DNSZoneRGNameDestroyDescription) _ = cmd.MarkFlagRequired("workload-identities-file") _ = cmd.MarkFlagRequired("azure-creds") _ = cmd.MarkFlagRequired("resource-group-name") _ = cmd.MarkFlagRequired("name") _ = cmd.MarkFlagRequired("infra-id") + _ = cmd.MarkFlagRequired("dns-zone-rg-name") l := log.Log cmd.RunE = func(cmd *cobra.Command, args []string) error { @@ -70,6 +72,7 @@ func BindDestroyIAMProductFlags(opts *DestroyIAMOptions, flags *pflag.FlagSet) { flags.StringVar(&opts.Cloud, "cloud", opts.Cloud, util.CloudDescription) flags.StringVar(&opts.Name, "name", opts.Name, util.NameDescription) flags.StringVar(&opts.InfraID, "infra-id", opts.InfraID, util.InfraIDDescription) + flags.StringVar(&opts.DNSZoneRG, "dns-zone-rg-name", opts.DNSZoneRG, "The resource group name where the DNS zone resides (used to clean up role assignments)") } // Validate validates the DestroyIAMOptions @@ -89,6 +92,9 @@ func (o *DestroyIAMOptions) Validate() error { if o.InfraID == "" { return fmt.Errorf("infra-id is required") } + if o.DNSZoneRG == "" { + return fmt.Errorf("dns-zone-rg-name is required") + } return nil } @@ -110,6 +116,19 @@ func (o *DestroyIAMOptions) Run(ctx context.Context, l logr.Logger) error { "infraID", o.InfraID, "resourceGroup", o.ResourceGroupName) + // Clean up role assignments before destroying identities to avoid orphans. + // Match the create path resource-group names: {name}-nsg and {name}-vnet. + nsgRG := o.Name + "-nsg" + vnetRG := o.Name + "-vnet" + + rbacManager := NewRBACManager(subscriptionID, azureCreds) + // assignCustomHCPRoles=false is safe: GetServicePrincipalScopes only uses the flag to select + // the role definition ID, not to modify the scopes list. Cleanup derives role assignment names + // from infraID + component + scope, so the role ID is irrelevant. + if err := rbacManager.CleanupRoleAssignments(ctx, l, o.InfraID, o.ResourceGroupName, nsgRG, vnetRG, o.DNSZoneRG, false); err != nil { + l.Error(err, "Failed to clean up some role assignments, continuing with identity deletion") + } + // Create the identity manager identityManager := NewIdentityManager(subscriptionID, azureCreds, o.Cloud) diff --git a/cmd/infra/azure/destroy_iam_test.go b/cmd/infra/azure/destroy_iam_test.go index da1d6ef5154b..9bbb0777fb5e 100644 --- a/cmd/infra/azure/destroy_iam_test.go +++ b/cmd/infra/azure/destroy_iam_test.go @@ -22,6 +22,7 @@ func TestDestroyIAMOptionsValidate(t *testing.T) { WorkloadIdentitiesFile: "/path/to/identities.json", CredentialsFile: "/path/to/creds.json", ResourceGroupName: "test-rg", + DNSZoneRG: "dns-zone-rg", }, expectedError: false, description: "Should pass when all required fields are provided", @@ -81,6 +82,18 @@ func TestDestroyIAMOptionsValidate(t *testing.T) { errorContains: "infra-id is required", description: "Should require infra-id", }, + "When dns-zone-rg-name is empty it should return an error": { + opts: DestroyIAMOptions{ + Name: "test-cluster", + InfraID: "test-infra-id", + WorkloadIdentitiesFile: "/path/to/identities.json", + CredentialsFile: "/path/to/creds.json", + ResourceGroupName: "test-rg", + }, + expectedError: true, + errorContains: "dns-zone-rg-name is required", + description: "Should require dns-zone-rg-name", + }, } for name, test := range tests { diff --git a/cmd/infra/azure/rbac.go b/cmd/infra/azure/rbac.go index f65694181890..2f72d35d412a 100644 --- a/cmd/infra/azure/rbac.go +++ b/cmd/infra/azure/rbac.go @@ -19,15 +19,26 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" azureauth "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" "k8s.io/utils/ptr" + + "github.com/go-logr/logr" ) const ( graphAPIEndpoint = "https://graph.microsoft.com/v1.0/servicePrincipals" ) +// roleAssignmentClient abstracts the Azure role assignment operations for testability. +type roleAssignmentClient interface { + Get(ctx context.Context, scope string, roleAssignmentName string, options *azureauth.RoleAssignmentsClientGetOptions) (azureauth.RoleAssignmentsClientGetResponse, error) + Delete(ctx context.Context, scope string, roleAssignmentName string, options *azureauth.RoleAssignmentsClientDeleteOptions) (azureauth.RoleAssignmentsClientDeleteResponse, error) + Create(ctx context.Context, scope string, roleAssignmentName string, parameters azureauth.RoleAssignmentCreateParameters, options *azureauth.RoleAssignmentsClientCreateOptions) (azureauth.RoleAssignmentsClientCreateResponse, error) + NewListForScopePager(scope string, options *azureauth.RoleAssignmentsClientListForScopeOptions) *runtime.Pager[azureauth.RoleAssignmentsClientListForScopeResponse] +} + // RBACManager handles Azure RBAC operations type RBACManager struct { subscriptionID string @@ -100,6 +111,11 @@ func (r *RBACManager) assignRolesForComponents(ctx context.Context, opts *Create return err } + raClient, err := azureauth.NewRoleAssignmentsClient(r.subscriptionID, r.creds, nil) + if err != nil { + return fmt.Errorf("failed to create role assignments client: %w", err) + } + for component, clientID := range components { objectID, err := r.getObjectIDFromClientID(string(clientID), token) if err != nil { @@ -109,7 +125,7 @@ func (r *RBACManager) assignRolesForComponents(ctx context.Context, opts *Create role, scopes := azureutil.GetServicePrincipalScopes(r.subscriptionID, resourceGroupName, nsgResourceGroupName, vnetResourceGroupName, opts.DNSZoneRG, component, opts.AssignCustomHCPRoles) for _, scope := range scopes { - if err := r.assignRole(ctx, opts.InfraID, component, objectID, role, scope); err != nil { + if err := r.assignRole(ctx, raClient, opts.InfraID, component, objectID, role, scope); err != nil { return fmt.Errorf("failed to perform role assignment: %w", err) } } @@ -128,12 +144,17 @@ func (r *RBACManager) AssignDataPlaneRoles(ctx context.Context, opts *CreateInfr return err } + raClient, err := azureauth.NewRoleAssignmentsClient(r.subscriptionID, r.creds, nil) + if err != nil { + return fmt.Errorf("failed to create role assignments client: %w", err) + } + // Setup Data Plane MI role assignments objectID, err := r.getObjectIDFromClientID(dataPlaneIdentities.ImageRegistryMSIClientID, token) if err != nil { return err } - err = r.assignRole(ctx, opts.InfraID, config.CIRO+"WI", objectID, config.ImageRegistryRoleDefinitionID, managedRG) + err = r.assignRole(ctx, raClient, opts.InfraID, config.CIRO+"WI", objectID, config.ImageRegistryRoleDefinitionID, managedRG) if err != nil { return err } @@ -142,7 +163,7 @@ func (r *RBACManager) AssignDataPlaneRoles(ctx context.Context, opts *CreateInfr if err != nil { return err } - err = r.assignRole(ctx, opts.InfraID, config.AzureDisk+"WI", objectID, config.AzureDiskRoleDefinitionID, managedRG) + err = r.assignRole(ctx, raClient, opts.InfraID, config.AzureDisk+"WI", objectID, config.AzureDiskRoleDefinitionID, managedRG) if err != nil { return err } @@ -151,7 +172,7 @@ func (r *RBACManager) AssignDataPlaneRoles(ctx context.Context, opts *CreateInfr if err != nil { return err } - err = r.assignRole(ctx, opts.InfraID, config.AzureFile+"WI", objectID, config.AzureFileRoleDefinitionID, managedRG) + err = r.assignRole(ctx, raClient, opts.InfraID, config.AzureFile+"WI", objectID, config.AzureFileRoleDefinitionID, managedRG) if err != nil { return err } @@ -160,12 +181,7 @@ func (r *RBACManager) AssignDataPlaneRoles(ctx context.Context, opts *CreateInfr } // assignRole assigns a scoped role to the service principal assignee -func (r *RBACManager) assignRole(ctx context.Context, infraID, component, assigneeID, role, scope string) error { - roleAssignmentClient, err := azureauth.NewRoleAssignmentsClient(r.subscriptionID, r.creds, nil) - if err != nil { - return fmt.Errorf("failed to create new role assignments client: %w", err) - } - +func (r *RBACManager) assignRole(ctx context.Context, client roleAssignmentClient, infraID, component, assigneeID, role, scope string) error { // Generate the role assignment name roleAssignmentName := util.GenerateRoleAssignmentName(infraID, component, scope) @@ -184,7 +200,7 @@ func (r *RBACManager) assignRole(ctx context.Context, infraID, component, assign // Robust existence check: // 1) List assignments for this principalId at or around this scope and // verify one matches both the exact scope and role definition ID. - pager := roleAssignmentClient.NewListForScopePager(scope, &azureauth.RoleAssignmentsClientListForScopeOptions{ + pager := client.NewListForScopePager(scope, &azureauth.RoleAssignmentsClientListForScopeOptions{ // Use atScope() to reliably list assignments at this scope, then match in code Filter: ptr.To("atScope()"), }) @@ -208,25 +224,47 @@ func (r *RBACManager) assignRole(ctx context.Context, infraID, component, assign } // 2) Fallback to a direct GET by our deterministic name; create only if 404. - _, err = roleAssignmentClient.Get(ctx, scope, roleAssignmentName, nil) + // If the assignment exists but points to a different principal (stale/orphaned from a + // previous cluster with the same infraID), delete it and fall through to create a new one. + existing, err := client.Get(ctx, scope, roleAssignmentName, nil) if err == nil { - log.Log.Info("Skipping role assignment creation, role assignment already exists.", "role", role, "assigneeID", assigneeID, "scope", scope) - return nil - } - var respErr *azcore.ResponseError - if errors.As(err, &respErr) { - if respErr.StatusCode == http.StatusNotFound { - // proceed to create - } else if respErr.StatusCode == http.StatusForbidden || strings.EqualFold(respErr.ErrorCode, "AuthorizationFailed") { - log.Log.Info("Get not permitted; will attempt create and rely on 409 for idempotency.", "role", role, "assigneeID", assigneeID, "scope", scope) - } else { - return fmt.Errorf("failed checking role assignment existence: %w", err) + if existing.Properties != nil && + existing.Properties.PrincipalID != nil && + existing.Properties.RoleDefinitionID != nil && + strings.EqualFold(*existing.Properties.PrincipalID, assigneeID) && + strings.EqualFold(*existing.Properties.RoleDefinitionID, roleDefinitionID) { + log.Log.Info("Skipping role assignment creation, role assignment already exists.", "role", role, "assigneeID", assigneeID, "scope", scope) + return nil + } + // Stale assignment — different principal owns this deterministic name. + stalePrincipal := "" + if existing.Properties != nil && existing.Properties.PrincipalID != nil { + stalePrincipal = *existing.Properties.PrincipalID } + log.Log.Info("Deleting stale role assignment with mismatched principal", + "role", role, "expectedPrincipal", assigneeID, + "stalePrincipal", stalePrincipal, + "scope", scope) + if _, err := client.Delete(ctx, scope, roleAssignmentName, nil); err != nil { + return fmt.Errorf("failed to delete stale role assignment: %w", err) + } + // Fall through to create a fresh assignment below. } else { - return fmt.Errorf("failed to check role assignment existence: %w", err) + var respErr *azcore.ResponseError + if errors.As(err, &respErr) { + if respErr.StatusCode == http.StatusNotFound { + // proceed to create + } else if respErr.StatusCode == http.StatusForbidden || strings.EqualFold(respErr.ErrorCode, "AuthorizationFailed") { + log.Log.Info("Get not permitted; will attempt create and rely on 409 for idempotency.", "role", role, "assigneeID", assigneeID, "scope", scope) + } else { + return fmt.Errorf("failed checking role assignment existence: %w", err) + } + } else { + return fmt.Errorf("failed to check role assignment existence: %w", err) + } } - _, err = roleAssignmentClient.Create(ctx, scope, roleAssignmentName, roleAssignmentProperties, nil) + _, err = client.Create(ctx, scope, roleAssignmentName, roleAssignmentProperties, nil) if err != nil { var respErr *azcore.ResponseError if errors.As(err, &respErr) && (respErr.StatusCode == http.StatusConflict || strings.EqualFold(respErr.ErrorCode, "RoleAssignmentExists")) { @@ -239,6 +277,84 @@ func (r *RBACManager) assignRole(ctx context.Context, infraID, component, assign return nil } +// CleanupRoleAssignments deletes all role assignments created for a cluster's workload identities. +// It regenerates the deterministic role assignment names from the infraID, component names, and scopes, +// then deletes each one. This must be called before destroying managed identities to avoid orphaned +// role assignments that cause naming collisions on re-creation. +func (r *RBACManager) CleanupRoleAssignments(ctx context.Context, l logr.Logger, infraID string, resourceGroupName, nsgResourceGroupName, vnetResourceGroupName, dnsZoneRG string, assignCustomHCPRoles bool) error { + raClient, err := azureauth.NewRoleAssignmentsClient(r.subscriptionID, r.creds, nil) + if err != nil { + return fmt.Errorf("failed to create role assignments client: %w", err) + } + return r.cleanupRoleAssignments(ctx, l, raClient, infraID, resourceGroupName, nsgResourceGroupName, vnetResourceGroupName, dnsZoneRG, assignCustomHCPRoles) +} + +// cleanupRoleAssignments is the testable inner method that performs the actual cleanup. +func (r *RBACManager) cleanupRoleAssignments(ctx context.Context, l logr.Logger, client roleAssignmentClient, infraID string, resourceGroupName, nsgResourceGroupName, vnetResourceGroupName, dnsZoneRG string, assignCustomHCPRoles bool) error { + // All components that may have role assignments + components := []string{ + config.CPO, + config.NodePoolMgmt, + config.CloudProvider, + config.AzureFile, + config.AzureDisk, + config.Ingress, + config.CNCC, + config.CIRO, + } + + // Data plane components use a "WI" suffix + dataPlaneComponents := []string{ + config.CIRO + "WI", + config.AzureDisk + "WI", + config.AzureFile + "WI", + } + + var deleteErrors []error + + // Cleanup control plane and workload identity role assignments + for _, component := range components { + _, scopes := azureutil.GetServicePrincipalScopes(r.subscriptionID, resourceGroupName, nsgResourceGroupName, vnetResourceGroupName, dnsZoneRG, component, assignCustomHCPRoles) + for _, scope := range scopes { + name := util.GenerateRoleAssignmentName(infraID, component, scope) + if err := r.deleteRoleAssignmentByName(ctx, l, client, scope, name, component); err != nil { + deleteErrors = append(deleteErrors, err) + } + } + } + + // Cleanup data plane role assignments (only scoped to managed RG) + managedRG := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s", r.subscriptionID, resourceGroupName) + for _, component := range dataPlaneComponents { + name := util.GenerateRoleAssignmentName(infraID, component, managedRG) + if err := r.deleteRoleAssignmentByName(ctx, l, client, managedRG, name, component); err != nil { + deleteErrors = append(deleteErrors, err) + } + } + + if len(deleteErrors) > 0 { + return fmt.Errorf("failed to delete %d role assignments during cleanup: %w", len(deleteErrors), errors.Join(deleteErrors...)) + } + + l.Info("Successfully cleaned up all role assignments", "infraID", infraID) + return nil +} + +func (r *RBACManager) deleteRoleAssignmentByName(ctx context.Context, l logr.Logger, client roleAssignmentClient, scope, name, component string) error { + _, err := client.Delete(ctx, scope, name, nil) + if err != nil { + var respErr *azcore.ResponseError + if errors.As(err, &respErr) && respErr.StatusCode == http.StatusNotFound { + l.Info("Role assignment not found, skipping", "component", component, "scope", scope) + return nil + } + l.Error(err, "Failed to delete role assignment", "component", component, "scope", scope) + return err + } + l.Info("Deleted role assignment", "component", component, "scope", scope) + return nil +} + func (r *RBACManager) getAzureToken() (azcore.AccessToken, error) { token, err := r.creds.GetToken(context.Background(), policy.TokenRequestOptions{ Scopes: []string{"https://graph.microsoft.com/.default"}, diff --git a/cmd/infra/azure/rbac_test.go b/cmd/infra/azure/rbac_test.go new file mode 100644 index 000000000000..d78fb514f42d --- /dev/null +++ b/cmd/infra/azure/rbac_test.go @@ -0,0 +1,510 @@ +package azure + +import ( + "context" + "fmt" + "net/http" + "sync" + "testing" + + . "github.com/onsi/gomega" + + "github.com/openshift/hypershift/cmd/util" + "github.com/openshift/hypershift/support/config" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + azureauth "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" + + "k8s.io/utils/ptr" + + "github.com/go-logr/logr" +) + +// mockRoleAssignmentClient implements roleAssignmentClient for testing. +type mockRoleAssignmentClient struct { + // getFunc is called when Get is invoked. + getFunc func(ctx context.Context, scope, name string, options *azureauth.RoleAssignmentsClientGetOptions) (azureauth.RoleAssignmentsClientGetResponse, error) + // deleteFunc is called when Delete is invoked. + deleteFunc func(ctx context.Context, scope, name string, options *azureauth.RoleAssignmentsClientDeleteOptions) (azureauth.RoleAssignmentsClientDeleteResponse, error) + // createFunc is called when Create is invoked. + createFunc func(ctx context.Context, scope, name string, params azureauth.RoleAssignmentCreateParameters, options *azureauth.RoleAssignmentsClientCreateOptions) (azureauth.RoleAssignmentsClientCreateResponse, error) + // listItems are returned by the pager from NewListForScopePager. + listItems []*azureauth.RoleAssignment + // listErr if set causes the pager to return this error. + listErr error +} + +func (m *mockRoleAssignmentClient) Get(ctx context.Context, scope, name string, options *azureauth.RoleAssignmentsClientGetOptions) (azureauth.RoleAssignmentsClientGetResponse, error) { + return m.getFunc(ctx, scope, name, options) +} + +func (m *mockRoleAssignmentClient) Delete(ctx context.Context, scope, name string, options *azureauth.RoleAssignmentsClientDeleteOptions) (azureauth.RoleAssignmentsClientDeleteResponse, error) { + return m.deleteFunc(ctx, scope, name, options) +} + +func (m *mockRoleAssignmentClient) Create(ctx context.Context, scope, name string, params azureauth.RoleAssignmentCreateParameters, options *azureauth.RoleAssignmentsClientCreateOptions) (azureauth.RoleAssignmentsClientCreateResponse, error) { + return m.createFunc(ctx, scope, name, params, options) +} + +func (m *mockRoleAssignmentClient) NewListForScopePager(_ string, _ *azureauth.RoleAssignmentsClientListForScopeOptions) *runtime.Pager[azureauth.RoleAssignmentsClientListForScopeResponse] { + items := m.listItems + listErr := m.listErr + return runtime.NewPager(runtime.PagingHandler[azureauth.RoleAssignmentsClientListForScopeResponse]{ + More: func(_ azureauth.RoleAssignmentsClientListForScopeResponse) bool { + return false + }, + Fetcher: func(_ context.Context, _ *azureauth.RoleAssignmentsClientListForScopeResponse) (azureauth.RoleAssignmentsClientListForScopeResponse, error) { + if listErr != nil { + return azureauth.RoleAssignmentsClientListForScopeResponse{}, listErr + } + return azureauth.RoleAssignmentsClientListForScopeResponse{ + RoleAssignmentListResult: azureauth.RoleAssignmentListResult{ + Value: items, + }, + }, nil + }, + }) +} + +func notFoundError() error { + return &azcore.ResponseError{ + StatusCode: http.StatusNotFound, + ErrorCode: "RoleAssignmentNotFound", + } +} + +func forbiddenError() error { + return &azcore.ResponseError{ + StatusCode: http.StatusForbidden, + ErrorCode: "AuthorizationFailed", + } +} + +func conflictError() error { + return &azcore.ResponseError{ + StatusCode: http.StatusConflict, + ErrorCode: "RoleAssignmentExists", + } +} + +func internalServerError() error { + return &azcore.ResponseError{ + StatusCode: http.StatusInternalServerError, + ErrorCode: "InternalServerError", + } +} + +func TestAssignRole(t *testing.T) { + const ( + subscriptionID = "test-sub-id" + infraID = "test-infra" + component = "ingress" + currentPrinc = "new-principal-id" + stalePrinc = "old-principal-id" + role = "0336e1d3-7a87-462b-b6db-342b63f7802c" + scope = "/subscriptions/test-sub-id/resourceGroups/test-rg" + ) + + roleDefID := "/subscriptions/" + subscriptionID + "/providers/Microsoft.Authorization/roleDefinitions/" + role + roleAssignmentName := util.GenerateRoleAssignmentName(infraID, component, scope) + + tests := map[string]struct { + listItems []*azureauth.RoleAssignment + listErr error + getResponse *azureauth.RoleAssignmentsClientGetResponse + getErr error + deleteErr error + createErr error + expectCreate bool + expectDelete bool + expectErr bool + }{ + // --- LIST behaviors --- + "When LIST finds matching assignment it should skip creation": { + listItems: []*azureauth.RoleAssignment{ + { + Properties: &azureauth.RoleAssignmentProperties{ + PrincipalID: ptr.To(currentPrinc), + RoleDefinitionID: ptr.To(roleDefID), + Scope: ptr.To(scope), + }, + }, + }, + getErr: notFoundError(), + expectCreate: false, + expectDelete: false, + }, + "When LIST returns items with nil properties it should skip them and fall through to GET": { + listItems: []*azureauth.RoleAssignment{ + {Properties: nil}, + {Properties: &azureauth.RoleAssignmentProperties{ + PrincipalID: nil, + RoleDefinitionID: ptr.To(roleDefID), + Scope: ptr.To(scope), + }}, + }, + getErr: notFoundError(), + expectCreate: true, + expectDelete: false, + }, + "When LIST page returns error it should return error": { + listErr: internalServerError(), + expectErr: true, + }, + + // --- GET behaviors --- + "When GET finds assignment with matching principal and role it should skip creation": { + getResponse: &azureauth.RoleAssignmentsClientGetResponse{ + RoleAssignment: azureauth.RoleAssignment{ + Properties: &azureauth.RoleAssignmentProperties{ + PrincipalID: ptr.To(currentPrinc), + RoleDefinitionID: ptr.To(roleDefID), + }, + }, + }, + expectCreate: false, + expectDelete: false, + }, + "When GET finds assignment with different principal it should delete stale and create new": { + getResponse: &azureauth.RoleAssignmentsClientGetResponse{ + RoleAssignment: azureauth.RoleAssignment{ + Properties: &azureauth.RoleAssignmentProperties{ + PrincipalID: ptr.To(stalePrinc), + }, + }, + }, + expectCreate: true, + expectDelete: true, + }, + "When GET finds assignment with nil PrincipalID it should delete stale and create new": { + getResponse: &azureauth.RoleAssignmentsClientGetResponse{ + RoleAssignment: azureauth.RoleAssignment{ + Properties: &azureauth.RoleAssignmentProperties{ + PrincipalID: nil, + }, + }, + }, + expectCreate: true, + expectDelete: true, + }, + "When GET finds assignment with nil Properties it should delete stale and create new": { + getResponse: &azureauth.RoleAssignmentsClientGetResponse{ + RoleAssignment: azureauth.RoleAssignment{ + Properties: nil, + }, + }, + expectCreate: true, + expectDelete: true, + }, + "When GET finds stale assignment but delete fails it should return error": { + getResponse: &azureauth.RoleAssignmentsClientGetResponse{ + RoleAssignment: azureauth.RoleAssignment{ + Properties: &azureauth.RoleAssignmentProperties{ + PrincipalID: ptr.To(stalePrinc), + }, + }, + }, + deleteErr: forbiddenError(), + expectDelete: true, + expectCreate: false, + expectErr: true, + }, + "When GET returns 404 it should create new assignment": { + getErr: notFoundError(), + expectCreate: true, + expectDelete: false, + }, + "When GET returns 403 it should fall through to create": { + getErr: forbiddenError(), + expectCreate: true, + expectDelete: false, + }, + "When GET returns unexpected API error it should return error": { + getErr: internalServerError(), + expectErr: true, + }, + "When GET returns non-API error it should return error": { + getErr: fmt.Errorf("network timeout"), + expectErr: true, + }, + + // --- Create behaviors --- + "When create returns 409 conflict it should succeed": { + getErr: notFoundError(), + createErr: conflictError(), + expectCreate: true, + expectDelete: false, + expectErr: false, + }, + "When create returns unexpected error it should return error": { + getErr: notFoundError(), + createErr: internalServerError(), + expectCreate: true, + expectDelete: false, + expectErr: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + g := NewWithT(t) + + var ( + created bool + deleted bool + createdName string + deletedName string + ) + + mock := &mockRoleAssignmentClient{ + listItems: tc.listItems, + listErr: tc.listErr, + getFunc: func(_ context.Context, _, _ string, _ *azureauth.RoleAssignmentsClientGetOptions) (azureauth.RoleAssignmentsClientGetResponse, error) { + if tc.getResponse != nil { + return *tc.getResponse, nil + } + return azureauth.RoleAssignmentsClientGetResponse{}, tc.getErr + }, + deleteFunc: func(_ context.Context, _, name string, _ *azureauth.RoleAssignmentsClientDeleteOptions) (azureauth.RoleAssignmentsClientDeleteResponse, error) { + deleted = true + deletedName = name + if tc.deleteErr != nil { + return azureauth.RoleAssignmentsClientDeleteResponse{}, tc.deleteErr + } + return azureauth.RoleAssignmentsClientDeleteResponse{}, nil + }, + createFunc: func(_ context.Context, _, name string, _ azureauth.RoleAssignmentCreateParameters, _ *azureauth.RoleAssignmentsClientCreateOptions) (azureauth.RoleAssignmentsClientCreateResponse, error) { + created = true + createdName = name + if tc.createErr != nil { + return azureauth.RoleAssignmentsClientCreateResponse{}, tc.createErr + } + return azureauth.RoleAssignmentsClientCreateResponse{}, nil + }, + } + + mgr := &RBACManager{subscriptionID: subscriptionID} + err := mgr.assignRole(t.Context(), mock, infraID, component, currentPrinc, role, scope) + + if tc.expectErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(created).To(Equal(tc.expectCreate), "create expectation mismatch") + g.Expect(deleted).To(Equal(tc.expectDelete), "delete expectation mismatch") + + if tc.expectCreate { + g.Expect(createdName).To(Equal(roleAssignmentName), "should use deterministic role assignment name") + } + if tc.expectDelete { + g.Expect(deletedName).To(Equal(roleAssignmentName), "should delete the stale role assignment by deterministic name") + } + }) + } +} + +func TestAssignRoleCreateParameters(t *testing.T) { + t.Run("When creating a new assignment it should pass correct principal ID, role definition, and scope", func(t *testing.T) { + g := NewWithT(t) + + const ( + subscriptionID = "test-sub-id" + infraID = "test-infra" + component = "ingress" + assigneeID = "principal-123" + role = "0336e1d3-7a87-462b-b6db-342b63f7802c" + scope = "/subscriptions/test-sub-id/resourceGroups/test-rg" + ) + + expectedRoleDefID := "/subscriptions/" + subscriptionID + "/providers/Microsoft.Authorization/roleDefinitions/" + role + expectedName := util.GenerateRoleAssignmentName(infraID, component, scope) + + var capturedParams azureauth.RoleAssignmentCreateParameters + var capturedScope, capturedName string + + mock := &mockRoleAssignmentClient{ + getFunc: func(_ context.Context, _, _ string, _ *azureauth.RoleAssignmentsClientGetOptions) (azureauth.RoleAssignmentsClientGetResponse, error) { + return azureauth.RoleAssignmentsClientGetResponse{}, notFoundError() + }, + deleteFunc: func(_ context.Context, _, _ string, _ *azureauth.RoleAssignmentsClientDeleteOptions) (azureauth.RoleAssignmentsClientDeleteResponse, error) { + return azureauth.RoleAssignmentsClientDeleteResponse{}, nil + }, + createFunc: func(_ context.Context, s, n string, params azureauth.RoleAssignmentCreateParameters, _ *azureauth.RoleAssignmentsClientCreateOptions) (azureauth.RoleAssignmentsClientCreateResponse, error) { + capturedScope = s + capturedName = n + capturedParams = params + return azureauth.RoleAssignmentsClientCreateResponse{}, nil + }, + } + + mgr := &RBACManager{subscriptionID: subscriptionID} + err := mgr.assignRole(t.Context(), mock, infraID, component, assigneeID, role, scope) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(capturedScope).To(Equal(scope)) + g.Expect(capturedName).To(Equal(expectedName)) + g.Expect(capturedParams.Properties).ToNot(BeNil()) + g.Expect(*capturedParams.Properties.PrincipalID).To(Equal(assigneeID)) + g.Expect(*capturedParams.Properties.RoleDefinitionID).To(Equal(expectedRoleDefID)) + g.Expect(*capturedParams.Properties.Scope).To(Equal(scope)) + }) +} + +func TestDeleteRoleAssignmentByName(t *testing.T) { + tests := map[string]struct { + deleteErr error + expectError bool + }{ + "When assignment exists it should delete successfully": { + deleteErr: nil, + expectError: false, + }, + "When assignment does not exist it should skip gracefully": { + deleteErr: notFoundError(), + expectError: false, + }, + "When delete fails with unexpected error it should return error": { + deleteErr: forbiddenError(), + expectError: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + g := NewWithT(t) + + mock := &mockRoleAssignmentClient{ + deleteFunc: func(_ context.Context, _, _ string, _ *azureauth.RoleAssignmentsClientDeleteOptions) (azureauth.RoleAssignmentsClientDeleteResponse, error) { + if tc.deleteErr != nil { + return azureauth.RoleAssignmentsClientDeleteResponse{}, tc.deleteErr + } + return azureauth.RoleAssignmentsClientDeleteResponse{}, nil + }, + } + + mgr := &RBACManager{subscriptionID: "test-sub"} + err := mgr.deleteRoleAssignmentByName(t.Context(), discardLogger(), mock, "/subscriptions/test-sub/resourceGroups/rg", "test-name", "test-component") + + if tc.expectError { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + }) + } +} + +// deleteCall records a single delete invocation for verification. +type deleteCall struct { + scope string + name string +} + +func TestCleanupRoleAssignments(t *testing.T) { + const ( + subscriptionID = "test-sub-id" + infraID = "test-infra" + managedRG = "test-rg" + nsgRG = "test-nsg-rg" + vnetRG = "test-vnet-rg" + dnsZoneRG = "os4-common" + ) + + managedRGScope := "/subscriptions/" + subscriptionID + "/resourceGroups/" + managedRG + dnsZoneScope := "/subscriptions/" + subscriptionID + "/resourceGroups/" + dnsZoneRG + vnetRGScope := "/subscriptions/" + subscriptionID + "/resourceGroups/" + vnetRG + + t.Run("When all assignments exist it should delete all control plane and data plane assignments", func(t *testing.T) { + g := NewWithT(t) + + var mu sync.Mutex + var calls []deleteCall + + mock := &mockRoleAssignmentClient{ + deleteFunc: func(_ context.Context, scope, name string, _ *azureauth.RoleAssignmentsClientDeleteOptions) (azureauth.RoleAssignmentsClientDeleteResponse, error) { + mu.Lock() + calls = append(calls, deleteCall{scope: scope, name: name}) + mu.Unlock() + return azureauth.RoleAssignmentsClientDeleteResponse{}, nil + }, + } + + mgr := &RBACManager{subscriptionID: subscriptionID} + err := mgr.cleanupRoleAssignments(t.Context(), discardLogger(), mock, infraID, managedRG, nsgRG, vnetRG, dnsZoneRG, false) + g.Expect(err).ToNot(HaveOccurred()) + + // Verify the ingress component's DNS zone scope is cleaned up (the exact bug scenario). + ingressDNSName := util.GenerateRoleAssignmentName(infraID, config.Ingress, dnsZoneScope) + g.Expect(calls).To(ContainElement(deleteCall{scope: dnsZoneScope, name: ingressDNSName}), + "should clean up ingress role assignment on DNS zone scope") + + // Verify the ingress component's vnet scope is cleaned up. + ingressVNetName := util.GenerateRoleAssignmentName(infraID, config.Ingress, vnetRGScope) + g.Expect(calls).To(ContainElement(deleteCall{scope: vnetRGScope, name: ingressVNetName}), + "should clean up ingress role assignment on vnet scope") + + // Verify data plane WI-suffixed components are cleaned up on managed RG. + for _, dp := range []string{config.CIRO + "WI", config.AzureDisk + "WI", config.AzureFile + "WI"} { + dpName := util.GenerateRoleAssignmentName(infraID, dp, managedRGScope) + g.Expect(calls).To(ContainElement(deleteCall{scope: managedRGScope, name: dpName}), + "should clean up data plane component %s", dp) + } + + // All 8 control plane components + 3 data plane components should produce delete calls. + // Exact count depends on GetServicePrincipalScopes; verify at least the minimum. + g.Expect(len(calls)).To(BeNumerically(">=", 11), + "should delete assignments for all components across their scopes") + }) + + t.Run("When some assignments are not found it should continue and succeed", func(t *testing.T) { + g := NewWithT(t) + + deleteCount := 0 + mock := &mockRoleAssignmentClient{ + deleteFunc: func(_ context.Context, _, _ string, _ *azureauth.RoleAssignmentsClientDeleteOptions) (azureauth.RoleAssignmentsClientDeleteResponse, error) { + deleteCount++ + // Every other call returns 404 + if deleteCount%2 == 0 { + return azureauth.RoleAssignmentsClientDeleteResponse{}, notFoundError() + } + return azureauth.RoleAssignmentsClientDeleteResponse{}, nil + }, + } + + mgr := &RBACManager{subscriptionID: subscriptionID} + err := mgr.cleanupRoleAssignments(t.Context(), discardLogger(), mock, infraID, managedRG, nsgRG, vnetRG, dnsZoneRG, false) + g.Expect(err).ToNot(HaveOccurred(), "404s should be treated as success") + g.Expect(deleteCount).To(BeNumerically(">=", 11), "should attempt all components even when some return 404") + }) + + t.Run("When some deletes fail with non-404 errors it should continue and return aggregate error", func(t *testing.T) { + g := NewWithT(t) + + deleteCount := 0 + failCount := 0 + mock := &mockRoleAssignmentClient{ + deleteFunc: func(_ context.Context, _, _ string, _ *azureauth.RoleAssignmentsClientDeleteOptions) (azureauth.RoleAssignmentsClientDeleteResponse, error) { + deleteCount++ + // Fail every 5th call with a non-404 error + if deleteCount%5 == 0 { + failCount++ + return azureauth.RoleAssignmentsClientDeleteResponse{}, forbiddenError() + } + return azureauth.RoleAssignmentsClientDeleteResponse{}, nil + }, + } + + mgr := &RBACManager{subscriptionID: subscriptionID} + err := mgr.cleanupRoleAssignments(t.Context(), discardLogger(), mock, infraID, managedRG, nsgRG, vnetRG, dnsZoneRG, false) + g.Expect(err).To(HaveOccurred(), "should return error when some deletes fail") + g.Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("failed to delete %d role assignments", failCount))) + g.Expect(deleteCount).To(BeNumerically(">=", 11), + "should attempt all components even when some fail") + }) +} + +func discardLogger() logr.Logger { + return logr.Discard() +} diff --git a/cmd/infra/azure/types.go b/cmd/infra/azure/types.go index 92b2456467c3..b4505d247f52 100644 --- a/cmd/infra/azure/types.go +++ b/cmd/infra/azure/types.go @@ -66,5 +66,6 @@ type DestroyIAMOptions struct { CredentialsFile string Credentials *util.AzureCreds ResourceGroupName string + DNSZoneRG string Cloud string } diff --git a/cmd/util/azure_flag_descriptions.go b/cmd/util/azure_flag_descriptions.go index 2ca6e49bb2b7..2d9b12d856cd 100644 --- a/cmd/util/azure_flag_descriptions.go +++ b/cmd/util/azure_flag_descriptions.go @@ -71,6 +71,7 @@ const ( LocationDestroyDescription = "Azure region of the cluster. Inferred from the HostedCluster if it exists; only required if the cluster resource has already been deleted." AzureCredsDestroyDescription = "Path to an Azure credentials file (JSON format) used to authenticate and delete Azure resources." ResourceGroupNameDestroyDescription = "Name of the resource group containing the cluster resources to delete. Inferred from the HostedCluster if it exists; only required if the cluster resource has already been deleted." + DNSZoneRGNameDestroyDescription = "Name of the resource group containing the Azure DNS zone (required). Used to clean up DNS zone role assignments during cluster or IAM destruction." // Infrastructure command specific flags AssignIdentityRolesDescription = "Automatically assign required Azure RBAC roles to workload identities. This grants the identities permissions to manage Azure resources." diff --git a/docs/content/how-to/azure/create-iam-separately.md b/docs/content/how-to/azure/create-iam-separately.md index cd59cd4f6c34..e392d0ec577b 100644 --- a/docs/content/how-to/azure/create-iam-separately.md +++ b/docs/content/how-to/azure/create-iam-separately.md @@ -157,7 +157,11 @@ To destroy the workload identities that were created: ```bash hypershift destroy iam azure \ --azure-creds AZURE_CREDENTIALS_FILE \ - --workload-identities-file workload-identities.json + --workload-identities-file workload-identities.json \ + --resource-group-name RESOURCE_GROUP \ + --name CLUSTER_NAME \ + --infra-id INFRA_ID \ + --dns-zone-rg-name DNS_ZONE_RG ``` The destroy command reads the output file from create to identify which identities to delete. @@ -194,6 +198,10 @@ Both the managed identities and their federated credentials are removed. |------|-------------| | `--azure-creds` | Path to Azure credentials JSON file | | `--workload-identities-file` | Path to workload identities JSON file | +| `--resource-group-name` | Resource group containing the identities | +| `--name` | Name of the HostedCluster | +| `--infra-id` | Unique infrastructure identifier | +| `--dns-zone-rg-name` | Resource group containing the Azure DNS zone | ### Optional Flags for `destroy iam azure` @@ -250,7 +258,10 @@ hypershift create cluster azure \ # --- Cleanup --- # 6. Destroy the cluster -hypershift destroy cluster azure --name ${NAME} +hypershift destroy cluster azure \ + --name ${NAME} \ + --azure-creds ${AZURE_CREDS} \ + --dns-zone-rg-name ${DNS_ZONE_RG} # 7. Destroy infrastructure hypershift destroy infra azure \ @@ -261,7 +272,11 @@ hypershift destroy infra azure \ # 8. Destroy IAM resources hypershift destroy iam azure \ --azure-creds ${AZURE_CREDS} \ - --workload-identities-file workload-identities.json + --workload-identities-file workload-identities.json \ + --resource-group-name ${RESOURCE_GROUP} \ + --name ${NAME} \ + --infra-id ${INFRA_ID} \ + --dns-zone-rg-name ${DNS_ZONE_RG} ``` ## See Also diff --git a/docs/content/how-to/azure/create-self-managed-azure-cluster.md b/docs/content/how-to/azure/create-self-managed-azure-cluster.md index 9263b993c3a8..6d213260b891 100644 --- a/docs/content/how-to/azure/create-self-managed-azure-cluster.md +++ b/docs/content/how-to/azure/create-self-managed-azure-cluster.md @@ -294,7 +294,8 @@ To delete the HostedCluster: hypershift destroy cluster azure \ --name $CLUSTER_NAME \ --azure-creds $AZURE_CREDS \ - --resource-group-name $MANAGED_RG_NAME + --resource-group-name $MANAGED_RG_NAME \ + --dns-zone-rg-name $PERSISTENT_RG_NAME ``` !!! note "Resource Cleanup" diff --git a/docs/content/how-to/azure/deploy-azure-private-clusters.md b/docs/content/how-to/azure/deploy-azure-private-clusters.md index 57a58ea4cd86..1184500fcdf2 100644 --- a/docs/content/how-to/azure/deploy-azure-private-clusters.md +++ b/docs/content/how-to/azure/deploy-azure-private-clusters.md @@ -389,7 +389,8 @@ To delete a private HostedCluster: hypershift destroy cluster azure \ --name ${CLUSTER_NAME} \ --azure-creds ${AZURE_CREDS} \ - --resource-group-name ${MANAGED_RG_NAME} + --resource-group-name ${MANAGED_RG_NAME} \ + --dns-zone-rg-name ${DNS_ZONE_RG_NAME} ``` The deletion process automatically cleans up Private Link resources in the correct order: diff --git a/docs/content/reference/aggregated-docs.md b/docs/content/reference/aggregated-docs.md index 09435523494f..ee120f8aa3d4 100644 --- a/docs/content/reference/aggregated-docs.md +++ b/docs/content/reference/aggregated-docs.md @@ -8912,7 +8912,11 @@ To destroy the workload identities that were created: ```bash hypershift destroy iam azure \ --azure-creds AZURE_CREDENTIALS_FILE \ - --workload-identities-file workload-identities.json + --workload-identities-file workload-identities.json \ + --resource-group-name RESOURCE_GROUP \ + --name CLUSTER_NAME \ + --infra-id INFRA_ID \ + --dns-zone-rg-name DNS_ZONE_RG ``` The destroy command reads the output file from create to identify which identities to delete. @@ -8949,6 +8953,10 @@ Both the managed identities and their federated credentials are removed. |------|-------------| | `--azure-creds` | Path to Azure credentials JSON file | | `--workload-identities-file` | Path to workload identities JSON file | +| `--resource-group-name` | Resource group containing the identities | +| `--name` | Name of the HostedCluster | +| `--infra-id` | Unique infrastructure identifier | +| `--dns-zone-rg-name` | Resource group containing the Azure DNS zone | ### Optional Flags for `destroy iam azure` @@ -9005,7 +9013,10 @@ hypershift create cluster azure \ # --- Cleanup --- # 6. Destroy the cluster -hypershift destroy cluster azure --name ${NAME} +hypershift destroy cluster azure \ + --name ${NAME} \ + --azure-creds ${AZURE_CREDS} \ + --dns-zone-rg-name ${DNS_ZONE_RG} # 7. Destroy infrastructure hypershift destroy infra azure \ @@ -9016,7 +9027,11 @@ hypershift destroy infra azure \ # 8. Destroy IAM resources hypershift destroy iam azure \ --azure-creds ${AZURE_CREDS} \ - --workload-identities-file workload-identities.json + --workload-identities-file workload-identities.json \ + --resource-group-name ${RESOURCE_GROUP} \ + --name ${NAME} \ + --infra-id ${INFRA_ID} \ + --dns-zone-rg-name ${DNS_ZONE_RG} ``` ## See Also @@ -9564,7 +9579,8 @@ To delete the HostedCluster: hypershift destroy cluster azure \ --name $CLUSTER_NAME \ --azure-creds $AZURE_CREDS \ - --resource-group-name $MANAGED_RG_NAME + --resource-group-name $MANAGED_RG_NAME \ + --dns-zone-rg-name $PERSISTENT_RG_NAME ``` !!! note "Resource Cleanup" @@ -9971,7 +9987,8 @@ To delete a private HostedCluster: hypershift destroy cluster azure \ --name ${CLUSTER_NAME} \ --azure-creds ${AZURE_CREDS} \ - --resource-group-name ${MANAGED_RG_NAME} + --resource-group-name ${MANAGED_RG_NAME} \ + --dns-zone-rg-name ${DNS_ZONE_RG_NAME} ``` The deletion process automatically cleans up Private Link resources in the correct order: diff --git a/product-cli/cmd/cluster/azure/destroy.go b/product-cli/cmd/cluster/azure/destroy.go index 1f14663895bd..a1b7589eab33 100644 --- a/product-cli/cmd/cluster/azure/destroy.go +++ b/product-cli/cmd/cluster/azure/destroy.go @@ -20,8 +20,10 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { cmd.Flags().StringVar(&opts.AzurePlatform.Location, "location", opts.AzurePlatform.Location, util.LocationDestroyDescription) cmd.Flags().StringVar(&opts.AzurePlatform.ResourceGroupName, "resource-group-name", opts.AzurePlatform.ResourceGroupName, util.ResourceGroupNameDestroyDescription) cmd.Flags().BoolVar(&opts.AzurePlatform.PreserveResourceGroup, "preserve-resource-group", opts.AzurePlatform.PreserveResourceGroup, util.PreserveResourceGroupDescription) + cmd.Flags().StringVar(&opts.AzurePlatform.DNSZoneRGName, "dns-zone-rg-name", opts.AzurePlatform.DNSZoneRGName, util.DNSZoneRGNameDestroyDescription) _ = cmd.MarkFlagRequired("azure-creds") + _ = cmd.MarkFlagRequired("dns-zone-rg-name") cmd.RunE = func(cmd *cobra.Command, args []string) error { return hypershiftazure.DestroyCluster(cmd.Context(), opts)