From 7f4751f222c215526f68a4a003f422f919d0e9d0 Mon Sep 17 00:00:00 2001 From: Rael Garcia Date: Thu, 27 Aug 2026 12:35:53 +0200 Subject: [PATCH 1/4] docs(slot-manager): document identity pool recovery (AROSLSRE-1896) --- test/cmd/aro-hcp-tests/slot-manager/DESIGN.md | 182 ++++++++++++++++++ 1 file changed, 182 insertions(+) diff --git a/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md b/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md index c2c4ea490b1..532c2ea2f85 100644 --- a/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md +++ b/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md @@ -182,6 +182,188 @@ flowchart TD - Current rollout limitation: - migration is still staged while legacy leases are retired, so the active slot inventory is being brought up gradually even though the code path now supports the broader multi-pool model. +### Identity pool reconciliation and recovery + +An E2E job can fail before cluster provisioning when its leased managed identity +container resource group is missing. The nested ARM error normally contains: + +```text +ResourceGroupNotFound: Resource group '' could not be found. +``` + +The canonical pool shape comes from `test/e2e-config/e2e-slots.yaml`. For each +pool, the slot manager expands: + +```text +-- +``` + +For example, a pool with `slot_count: 5` and +`identity_container_count: 60` expects slot suffixes `00` through `04`, each +with container suffixes `00` through `59`. + +#### Reconcile a pool + +Use the Make target rather than invoking `go run` or a previously built binary. +The target regenerates the Bicep-derived ARM template before rebuilding +`aro-hcp-tests`, preventing a stale embedded `msi-pools.json` from being +applied. + +```bash +make -C test apply-identity-pool \ + ENVIRONMENT= \ + SUBSCRIPTION="" +``` + +`SUBSCRIPTION` limits the operation to matching catalog pools. Omitting it +reconciles every managed pool in the selected environment and skips pools with +`identity_provisioning: unmanaged`. Supplying it can include a matching +unmanaged pool, so only do that when the subscription owner intends to manage +that pool with this command. + +The command applies one subscription-scoped deployment stack per slot. Each +stack creates or updates the slot's resource groups and the 13 well-known user +assigned identities in every group: + +```text +cluster-api-azure +control-plane +cloud-controller-manager +ingress +disk-csi-driver +file-csi-driver +image-registry +cloud-network-config +kms +dp-disk-csi-driver +dp-file-csi-driver +dp-image-registry +service +``` + +Before applying, confirm that the selected Azure credential can resolve the +catalog subscription and has permission to create subscription deployment +stacks, resource groups, and managed identities. Also confirm required resource +providers and subscription quotas are available. + +Deployment stacks use `ActionOnUnmanage: delete` for resources and resource +groups. A later catalog change that reduces a pool or changes its naming can +therefore delete resources no longer managed by the stack. Review the catalog +diff and always scope a recovery to the intended subscription. + +#### Validate the complete pool + +Do not validate only the resource group named in the original failure. Generate +the full expected inventory from the catalog and compare it with Azure. The +following commands are read-only: + +```bash +environment=dev +subscription="ARO HCP E2E Hosted Clusters (EA Subscription)" +catalog=test/e2e-config/e2e-slots.yaml + +pools="$( + ENVIRONMENT="$environment" SUBSCRIPTION_NAME="$subscription" \ + yq -o=json \ + '[.environments[strenv(ENVIRONMENT)].pools[] | + select(.subscription_name == strenv(SUBSCRIPTION_NAME))]' \ + "$catalog" +)" + +if (( $(jq 'length' <<<"$pools") == 0 )); then + echo "no matching pools found in $environment for $subscription" >&2 + exit 1 +fi + +workdir="$(mktemp -d)" +trap 'rm -rf "$workdir"' EXIT +: >"$workdir/expected-groups" +: >"$workdir/actual-groups" + +while IFS= read -r pool; do + prefix="$(jq -r '.identity_container_prefix' <<<"$pool")" + slot_count="$(jq -r '.slot_count' <<<"$pool")" + container_count="$(jq -r '.identity_container_count' <<<"$pool")" + + for ((slot = 0; slot < slot_count; slot++)); do + for ((container = 0; container < container_count; container++)); do + printf '%s-%02d-%02d\n' "$prefix" "$slot" "$container" + done + done >>"$workdir/expected-groups" + + az group list \ + --subscription "$subscription" \ + --query "[?starts_with(name, '$prefix-')].name" \ + --output tsv >>"$workdir/actual-groups" +done < <(jq -c '.[]' <<<"$pools") + +sort -u -o "$workdir/expected-groups" "$workdir/expected-groups" +sort -u -o "$workdir/actual-groups" "$workdir/actual-groups" + +comm -23 "$workdir/expected-groups" "$workdir/actual-groups" +``` + +No output from `comm` means every expected resource group exists. Compare the +counts as an additional summary: + +```bash +printf 'expected=%s actual=%s missing=%s\n' \ + "$(wc -l <"$workdir/expected-groups" | tr -d ' ')" \ + "$(wc -l <"$workdir/actual-groups" | tr -d ' ')" \ + "$(comm -23 "$workdir/expected-groups" "$workdir/actual-groups" | + wc -l | tr -d ' ')" +``` + +Validate the identity names across the complete pool with one subscription +resource-list query: + +```bash +identity_names=( + cluster-api-azure + control-plane + cloud-controller-manager + ingress + disk-csi-driver + file-csi-driver + image-registry + cloud-network-config + kms + dp-disk-csi-driver + dp-file-csi-driver + dp-image-registry + service +) + +while IFS= read -r resource_group; do + for identity_name in "${identity_names[@]}"; do + printf '%s\t%s\n' "$resource_group" "$identity_name" + done +done <"$workdir/expected-groups" | + sort >"$workdir/expected-identities" + +: >"$workdir/actual-identities" +while IFS= read -r prefix; do + az resource list \ + --subscription "$subscription" \ + --resource-type Microsoft.ManagedIdentity/userAssignedIdentities \ + --query "[?starts_with(resourceGroup, '$prefix-')].[resourceGroup,name]" \ + --output tsv >>"$workdir/actual-identities" +done < <(jq -r '.[].identity_container_prefix' <<<"$pools" | sort -u) + +sort -u -o "$workdir/actual-identities" "$workdir/actual-identities" + +comm -23 "$workdir/expected-identities" "$workdir/actual-identities" +``` + +No output means every expected identity exists. If reconciliation fails, retain +the deployment stack error and inspect the first nested Azure error rather than +retrying blindly. Common blockers are insufficient RBAC, an unregistered +`Microsoft.ManagedIdentity` provider, subscription quota exhaustion, or another +deployment operation holding the stack in a non-terminal state. + +This recovery procedure was added after +[AROSLSRE-1895](https://redhat.atlassian.net/browse/AROSLSRE-1895). + ### Dev subscription onboarding note - Adding a new **dev** customer subscription to the slot catalog is **not** sufficient by itself. From 91c6776aae2090eed834ce3a0eb1022d4525cb5b Mon Sep 17 00:00:00 2001 From: Rael Garcia Date: Thu, 27 Aug 2026 12:47:12 +0200 Subject: [PATCH 2/4] feat(slot-manager): validate identity pool inventory (AROSLSRE-1896) --- test/Makefile | 9 + test/cmd/aro-hcp-tests/slot-manager/DESIGN.md | 125 +----- test/cmd/aro-hcp-tests/slot-manager/cmd.go | 5 + .../slot-manager/identity-pool/cmd.go | 40 ++ .../slot-manager/identity-pool/pools.go | 26 +- .../slot-manager/identity-pool/validate.go | 403 ++++++++++++++++++ .../identity-pool/validate_test.go | 140 ++++++ 7 files changed, 633 insertions(+), 115 deletions(-) create mode 100644 test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate.go create mode 100644 test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go diff --git a/test/Makefile b/test/Makefile index 2ef7db2a499..db763e7e10d 100644 --- a/test/Makefile +++ b/test/Makefile @@ -56,6 +56,15 @@ apply-identity-pool: $(ARO_HCP_TESTS) $(if $(SUBSCRIPTION),--subscription "$(SUBSCRIPTION)") .PHONY: apply-identity-pool +# Validate the slot-managed identity pool against the canonical catalog using +# read-only Azure list operations. SUBSCRIPTION has the same filtering semantics +# as apply-identity-pool. +validate-identity-pool: $(ARO_HCP_TESTS) + $(ARO_HCP_TESTS) slot-manager validate-identity-pool \ + --environment "$${ENVIRONMENT:?ENVIRONMENT must be set (e.g. dev, int, stg, prod)}" \ + $(if $(SUBSCRIPTION),--subscription "$(SUBSCRIPTION)") +.PHONY: validate-identity-pool + int-e2e: PROW_JOB_NAME="$$(yq .clouds.public.environments.int.defaults.e2e.regionTest.prowJobName < ../config/config.msft.clouds-overlay.yaml)" \ REGION="uksouth" \ diff --git a/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md b/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md index 532c2ea2f85..f22203ea990 100644 --- a/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md +++ b/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md @@ -253,113 +253,32 @@ diff and always scope a recovery to the intended subscription. #### Validate the complete pool -Do not validate only the resource group named in the original failure. Generate -the full expected inventory from the catalog and compare it with Azure. The -following commands are read-only: +Do not validate only the resource group named in the original failure. Use the +read-only validation target to compare the complete slot-expanded catalog +inventory with Azure: ```bash -environment=dev -subscription="ARO HCP E2E Hosted Clusters (EA Subscription)" -catalog=test/e2e-config/e2e-slots.yaml - -pools="$( - ENVIRONMENT="$environment" SUBSCRIPTION_NAME="$subscription" \ - yq -o=json \ - '[.environments[strenv(ENVIRONMENT)].pools[] | - select(.subscription_name == strenv(SUBSCRIPTION_NAME))]' \ - "$catalog" -)" - -if (( $(jq 'length' <<<"$pools") == 0 )); then - echo "no matching pools found in $environment for $subscription" >&2 - exit 1 -fi - -workdir="$(mktemp -d)" -trap 'rm -rf "$workdir"' EXIT -: >"$workdir/expected-groups" -: >"$workdir/actual-groups" - -while IFS= read -r pool; do - prefix="$(jq -r '.identity_container_prefix' <<<"$pool")" - slot_count="$(jq -r '.slot_count' <<<"$pool")" - container_count="$(jq -r '.identity_container_count' <<<"$pool")" - - for ((slot = 0; slot < slot_count; slot++)); do - for ((container = 0; container < container_count; container++)); do - printf '%s-%02d-%02d\n' "$prefix" "$slot" "$container" - done - done >>"$workdir/expected-groups" - - az group list \ - --subscription "$subscription" \ - --query "[?starts_with(name, '$prefix-')].name" \ - --output tsv >>"$workdir/actual-groups" -done < <(jq -c '.[]' <<<"$pools") - -sort -u -o "$workdir/expected-groups" "$workdir/expected-groups" -sort -u -o "$workdir/actual-groups" "$workdir/actual-groups" - -comm -23 "$workdir/expected-groups" "$workdir/actual-groups" -``` - -No output from `comm` means every expected resource group exists. Compare the -counts as an additional summary: - -```bash -printf 'expected=%s actual=%s missing=%s\n' \ - "$(wc -l <"$workdir/expected-groups" | tr -d ' ')" \ - "$(wc -l <"$workdir/actual-groups" | tr -d ' ')" \ - "$(comm -23 "$workdir/expected-groups" "$workdir/actual-groups" | - wc -l | tr -d ' ')" -``` - -Validate the identity names across the complete pool with one subscription -resource-list query: - -```bash -identity_names=( - cluster-api-azure - control-plane - cloud-controller-manager - ingress - disk-csi-driver - file-csi-driver - image-registry - cloud-network-config - kms - dp-disk-csi-driver - dp-file-csi-driver - dp-image-registry - service -) - -while IFS= read -r resource_group; do - for identity_name in "${identity_names[@]}"; do - printf '%s\t%s\n' "$resource_group" "$identity_name" - done -done <"$workdir/expected-groups" | - sort >"$workdir/expected-identities" - -: >"$workdir/actual-identities" -while IFS= read -r prefix; do - az resource list \ - --subscription "$subscription" \ - --resource-type Microsoft.ManagedIdentity/userAssignedIdentities \ - --query "[?starts_with(resourceGroup, '$prefix-')].[resourceGroup,name]" \ - --output tsv >>"$workdir/actual-identities" -done < <(jq -r '.[].identity_container_prefix' <<<"$pools" | sort -u) - -sort -u -o "$workdir/actual-identities" "$workdir/actual-identities" - -comm -23 "$workdir/expected-identities" "$workdir/actual-identities" +make -C test validate-identity-pool \ + ENVIRONMENT= \ + SUBSCRIPTION="" ``` -No output means every expected identity exists. If reconciliation fails, retain -the deployment stack error and inspect the first nested Azure error rather than -retrying blindly. Common blockers are insufficient RBAC, an unregistered -`Microsoft.ManagedIdentity` provider, subscription quota exhaustion, or another -deployment operation holding the stack in a non-terminal state. +The target uses bulk Azure list operations and does not create, update, or +delete resources. It validates every matching catalog pool in the subscription, +including the complete resource-group inventory and the exact 13 identity names +in every existing expected group. It reports sorted missing and unexpected +resources, prints per-subscription counts, and exits non-zero when drift is +detected. + +`SUBSCRIPTION` has the same selection semantics as the apply target. Omitting it +validates every managed pool in the environment; setting it limits validation to +matching pools and can include an externally managed pool. + +If reconciliation or validation fails, retain the deployment stack error and +inspect the first nested Azure error rather than retrying blindly. Common +blockers are insufficient RBAC, an unregistered `Microsoft.ManagedIdentity` +provider, subscription quota exhaustion, or another deployment operation +holding the stack in a non-terminal state. This recovery procedure was added after [AROSLSRE-1895](https://redhat.atlassian.net/browse/AROSLSRE-1895). diff --git a/test/cmd/aro-hcp-tests/slot-manager/cmd.go b/test/cmd/aro-hcp-tests/slot-manager/cmd.go index 23e9adb48f4..c4f8ec73108 100644 --- a/test/cmd/aro-hcp-tests/slot-manager/cmd.go +++ b/test/cmd/aro-hcp-tests/slot-manager/cmd.go @@ -58,11 +58,16 @@ func NewCommand() (*cobra.Command, error) { if err != nil { return nil, err } + validateIdentityPoolCommand, err := identitypool.NewValidateCommand() + if err != nil { + return nil, err + } cmd.AddCommand(acquireCommand) cmd.AddCommand(releaseCommand) cmd.AddCommand(syncBoskosConfigCommand) cmd.AddCommand(validateBoskosConfigCommand) cmd.AddCommand(applyIdentityPoolCommand) + cmd.AddCommand(validateIdentityPoolCommand) return cmd, nil } diff --git a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/cmd.go b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/cmd.go index 0bce6a1ad1b..5004ca76931 100644 --- a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/cmd.go +++ b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/cmd.go @@ -52,6 +52,34 @@ subscription name and region. return cmd, nil } +func NewValidateCommand() (*cobra.Command, error) { + opts := DefaultValidateOptions() + + cmd := &cobra.Command{ + Use: "validate-identity-pool", + Short: "Validate the managed identity pool against the canonical slot catalog.", + Long: `Validate the managed identity pool against the canonical slot catalog. + +This command performs read-only Azure list operations and compares the resource groups and user-assigned identities in each +selected subscription with the slot-expanded inventory declared by the canonical E2E slot catalog. + +Validation covers every matching pool and fails when resource groups or expected identities are missing, or when unexpected +resource groups or identities are present within a managed pool prefix. +`, + SilenceUsage: true, + } + + if err := BindValidateOptions(opts, cmd); err != nil { + return nil, err + } + cmd.RunE = func(cmd *cobra.Command, args []string) error { + opts.Out = cmd.OutOrStdout() + return Validate(cmd.Context(), opts) + } + + return cmd, nil +} + func Apply(ctx context.Context, opts *RawApplyOptions) error { validated, err := opts.Validate() if err != nil { @@ -63,3 +91,15 @@ func Apply(ctx context.Context, opts *RawApplyOptions) error { } return completed.Run(ctx) } + +func Validate(ctx context.Context, opts *RawValidateOptions) error { + validated, err := opts.Validate() + if err != nil { + return err + } + completed, err := validated.Complete(ctx) + if err != nil { + return err + } + return completed.Run(ctx) +} diff --git a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/pools.go b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/pools.go index 33614df876b..6804ada03da 100644 --- a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/pools.go +++ b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/pools.go @@ -28,12 +28,13 @@ import ( type subscriptionIDResolverFunc func(ctx context.Context, name string) (string, error) type identityPool struct { - Environment string - Region string - ProvisioningRegion string - SubscriptionName string - SubscriptionID string - Slots []slots.ExpandedSlot + Environment string + Region string + ProvisioningRegion string + SubscriptionName string + SubscriptionID string + IdentityContainerPrefix string + Slots []slots.ExpandedSlot } // loadIdentityPools loads pools for the given environment. When @@ -85,12 +86,13 @@ func loadIdentityPools(ctx context.Context, catalogPath, environment string, sub } pools = append(pools, identityPool{ - Environment: environment, - Region: pool.Region, - ProvisioningRegion: pool.EffectiveIdentityProvisioningRegion(), - SubscriptionName: pool.SubscriptionName, - SubscriptionID: subscriptionID, - Slots: slots.ExpandSlotsForPool(environment, pool), + Environment: environment, + Region: pool.Region, + ProvisioningRegion: pool.EffectiveIdentityProvisioningRegion(), + SubscriptionName: pool.SubscriptionName, + SubscriptionID: subscriptionID, + IdentityContainerPrefix: pool.IdentityContainerPrefix, + Slots: slots.ExpandSlotsForPool(environment, pool), }) } diff --git a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate.go b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate.go new file mode 100644 index 00000000000..ab74fb7ba21 --- /dev/null +++ b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate.go @@ -0,0 +1,403 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package identitypool + +import ( + "context" + "fmt" + "io" + "sort" + "strings" + + "github.com/spf13/cobra" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/msi/armmsi" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources" + + "github.com/Azure/ARO-HCP/test/util/framework" +) + +type RawValidateOptions struct { + Environment string + SlotCatalog string + Subscriptions []string + Out io.Writer +} + +type validatedValidateOptions struct { + *RawValidateOptions +} + +type ValidatedValidateOptions struct { + *validatedValidateOptions +} + +type subscriptionInventory struct { + ResourceGroups []string + Identities map[string][]string +} + +type inventoryLoaderFunc func(ctx context.Context, subscriptionID string) (subscriptionInventory, error) + +type completedValidateOptions struct { + IdentityPools []identityPool + LoadInventory inventoryLoaderFunc + Out io.Writer +} + +type ValidateOptions struct { + *completedValidateOptions +} + +type identityReference struct { + ResourceGroup string + Name string +} + +type validationResult struct { + SubscriptionName string + ExpectedResourceGroups int + ActualResourceGroups int + ExpectedIdentities int + ActualIdentities int + MissingResourceGroups []string + UnexpectedResourceGroups []string + MissingIdentities []identityReference + UnexpectedIdentities []identityReference +} + +func DefaultValidateOptions() *RawValidateOptions { + return &RawValidateOptions{} +} + +func BindValidateOptions(opts *RawValidateOptions, cmd *cobra.Command) error { + cmd.Flags().StringVar(&opts.Environment, "environment", opts.Environment, "Environment short name. One of: int, stg, dev, prod") + cmd.Flags().StringVar(&opts.SlotCatalog, "slot-catalog", opts.SlotCatalog, "Path to the canonical E2E slot catalog") + cmd.Flags().StringSliceVar(&opts.Subscriptions, "subscription", opts.Subscriptions, "Limit validation to the named subscription(s). When set, unmanaged pools matching the filter are included.") + if err := cmd.MarkFlagRequired("environment"); err != nil { + return fmt.Errorf("failed to mark flag %q as required: %w", "environment", err) + } + return nil +} + +func (o *RawValidateOptions) Validate() (*ValidatedValidateOptions, error) { + if o.Environment == "" { + return nil, fmt.Errorf("--environment must not be empty") + } + if o.Out == nil { + o.Out = io.Discard + } + + return &ValidatedValidateOptions{ + validatedValidateOptions: &validatedValidateOptions{ + RawValidateOptions: o, + }, + }, nil +} + +func (o *ValidatedValidateOptions) Complete(ctx context.Context) (*ValidateOptions, error) { + tc := framework.NewTestContext() + cred, err := tc.AzureCredential() + if err != nil { + return nil, fmt.Errorf("failed getting Azure credential: %w", err) + } + + subscriptionClientFactory, err := tc.GetARMSubscriptionsClientFactory() + if err != nil { + return nil, fmt.Errorf("failed getting ARM subscriptions client factory: %w", err) + } + subscriptionClient := subscriptionClientFactory.NewClient() + + pools, err := loadIdentityPools(ctx, o.SlotCatalog, o.Environment, o.Subscriptions, func(ctx context.Context, name string) (string, error) { + return framework.GetSubscriptionID(ctx, subscriptionClient, name) + }) + if err != nil { + return nil, fmt.Errorf("failed loading identity pools from slot catalog: %w", err) + } + if len(pools) == 0 { + return nil, fmt.Errorf("no identity pools matched environment %q and the requested subscription filter", o.Environment) + } + + return &ValidateOptions{ + completedValidateOptions: &completedValidateOptions{ + IdentityPools: pools, + LoadInventory: func(ctx context.Context, subscriptionID string) (subscriptionInventory, error) { + return loadSubscriptionInventory(ctx, subscriptionID, cred) + }, + Out: o.Out, + }, + }, nil +} + +func (o *ValidateOptions) Run(ctx context.Context) error { + poolsBySubscription := map[string][]identityPool{} + subscriptionOrder := make([]string, 0) + for _, pool := range o.IdentityPools { + if _, found := poolsBySubscription[pool.SubscriptionID]; !found { + subscriptionOrder = append(subscriptionOrder, pool.SubscriptionID) + } + poolsBySubscription[pool.SubscriptionID] = append(poolsBySubscription[pool.SubscriptionID], pool) + } + + failedSubscriptions := 0 + for _, subscriptionID := range subscriptionOrder { + pools := poolsBySubscription[subscriptionID] + inventory, err := o.LoadInventory(ctx, subscriptionID) + if err != nil { + return fmt.Errorf("failed loading Azure identity-pool inventory for subscription %q: %w", pools[0].SubscriptionName, err) + } + + result := compareIdentityPoolInventory(pools, inventory) + writeValidationResult(o.Out, result) + if !result.valid() { + failedSubscriptions++ + } + } + + if failedSubscriptions > 0 { + return fmt.Errorf("identity pool validation failed for %d subscription(s)", failedSubscriptions) + } + return nil +} + +func loadSubscriptionInventory(ctx context.Context, subscriptionID string, cred azcore.TokenCredential) (subscriptionInventory, error) { + resourcesFactory, err := armresources.NewClientFactory(subscriptionID, cred, nil) + if err != nil { + return subscriptionInventory{}, fmt.Errorf("failed creating resources client factory: %w", err) + } + + resourceGroups := make([]string, 0) + resourceGroupsPager := resourcesFactory.NewResourceGroupsClient().NewListPager(nil) + for resourceGroupsPager.More() { + page, err := resourceGroupsPager.NextPage(ctx) + if err != nil { + return subscriptionInventory{}, fmt.Errorf("failed listing resource groups: %w", err) + } + for _, resourceGroup := range page.Value { + if resourceGroup.Name == nil { + return subscriptionInventory{}, fmt.Errorf("resource group list returned an entry without a name") + } + resourceGroups = append(resourceGroups, *resourceGroup.Name) + } + } + + msiFactory, err := armmsi.NewClientFactory(subscriptionID, cred, nil) + if err != nil { + return subscriptionInventory{}, fmt.Errorf("failed creating managed identity client factory: %w", err) + } + + identities := map[string][]string{} + identitiesPager := msiFactory.NewUserAssignedIdentitiesClient().NewListBySubscriptionPager(nil) + for identitiesPager.More() { + page, err := identitiesPager.NextPage(ctx) + if err != nil { + return subscriptionInventory{}, fmt.Errorf("failed listing user-assigned identities: %w", err) + } + for _, identity := range page.Value { + if identity.ID == nil || identity.Name == nil { + return subscriptionInventory{}, fmt.Errorf("managed identity list returned an entry without an ID or name") + } + resourceID, err := azcorearm.ParseResourceID(*identity.ID) + if err != nil { + return subscriptionInventory{}, fmt.Errorf("failed parsing managed identity resource ID %q: %w", *identity.ID, err) + } + identities[resourceID.ResourceGroupName] = append(identities[resourceID.ResourceGroupName], *identity.Name) + } + } + + return subscriptionInventory{ + ResourceGroups: resourceGroups, + Identities: identities, + }, nil +} + +func compareIdentityPoolInventory(pools []identityPool, actual subscriptionInventory) validationResult { + expectedResourceGroups := map[string]string{} + managedPrefixes := make([]string, 0) + for _, pool := range pools { + managedPrefixes = append(managedPrefixes, normalizeName(pool.IdentityContainerPrefix)+"-") + for _, slot := range pool.Slots { + for _, resourceGroup := range slot.IdentityContainerNames() { + expectedResourceGroups[normalizeName(resourceGroup)] = resourceGroup + } + } + } + + actualResourceGroups := map[string]string{} + for _, resourceGroup := range actual.ResourceGroups { + if hasAnyPrefix(resourceGroup, managedPrefixes) { + actualResourceGroups[normalizeName(resourceGroup)] = resourceGroup + } + } + + expectedIdentityNames := framework.NewDefaultIdentities().ToSlice() + expectedIdentities := map[identityReference]identityReference{} + actualIdentities := map[identityReference]identityReference{} + actualIdentitiesByResourceGroup := map[string][]string{} + for resourceGroup, identityNames := range actual.Identities { + normalizedResourceGroup := normalizeName(resourceGroup) + actualIdentitiesByResourceGroup[normalizedResourceGroup] = append(actualIdentitiesByResourceGroup[normalizedResourceGroup], identityNames...) + } + for normalizedResourceGroup, resourceGroup := range expectedResourceGroups { + for _, identityName := range expectedIdentityNames { + reference := identityReference{ResourceGroup: resourceGroup, Name: identityName} + expectedIdentities[normalizeIdentityReference(reference)] = reference + } + actualResourceGroup, found := actualResourceGroups[normalizedResourceGroup] + if !found { + continue + } + for _, identityName := range actualIdentitiesByResourceGroup[normalizedResourceGroup] { + reference := identityReference{ResourceGroup: actualResourceGroup, Name: identityName} + actualIdentities[normalizeIdentityReference(reference)] = reference + } + } + + return validationResult{ + SubscriptionName: pools[0].SubscriptionName, + ExpectedResourceGroups: len(expectedResourceGroups), + ActualResourceGroups: len(actualResourceGroups), + ExpectedIdentities: len(expectedIdentities), + ActualIdentities: len(actualIdentities), + MissingResourceGroups: differenceNamedStrings(expectedResourceGroups, actualResourceGroups), + UnexpectedResourceGroups: differenceNamedStrings(actualResourceGroups, expectedResourceGroups), + MissingIdentities: differenceIdentitiesForExistingGroups(expectedIdentities, actualIdentities, actualResourceGroups), + UnexpectedIdentities: differenceIdentities(actualIdentities, expectedIdentities), + } +} + +func (r validationResult) valid() bool { + return len(r.MissingResourceGroups) == 0 && + len(r.UnexpectedResourceGroups) == 0 && + len(r.MissingIdentities) == 0 && + len(r.UnexpectedIdentities) == 0 +} + +func writeValidationResult(out io.Writer, result validationResult) { + fmt.Fprintf(out, "subscription %q:\n", result.SubscriptionName) + fmt.Fprintf( + out, + " resource groups: expected=%d actual=%d missing=%d unexpected=%d\n", + result.ExpectedResourceGroups, + result.ActualResourceGroups, + len(result.MissingResourceGroups), + len(result.UnexpectedResourceGroups), + ) + fmt.Fprintf( + out, + " managed identities: expected=%d actual=%d missing_in_existing_groups=%d unexpected=%d\n", + result.ExpectedIdentities, + result.ActualIdentities, + len(result.MissingIdentities), + len(result.UnexpectedIdentities), + ) + writeStringList(out, "missing resource groups", result.MissingResourceGroups) + writeStringList(out, "unexpected resource groups", result.UnexpectedResourceGroups) + writeIdentityList(out, "missing identities in existing resource groups", result.MissingIdentities) + writeIdentityList(out, "unexpected identities", result.UnexpectedIdentities) + if result.valid() { + fmt.Fprintln(out, " result: valid") + } else { + fmt.Fprintln(out, " result: drift detected") + } +} + +func writeStringList(out io.Writer, title string, values []string) { + if len(values) == 0 { + return + } + fmt.Fprintf(out, " %s:\n", title) + for _, value := range values { + fmt.Fprintf(out, " - %s\n", value) + } +} + +func writeIdentityList(out io.Writer, title string, values []identityReference) { + if len(values) == 0 { + return + } + fmt.Fprintf(out, " %s:\n", title) + for _, value := range values { + fmt.Fprintf(out, " - %s/%s\n", value.ResourceGroup, value.Name) + } +} + +func hasAnyPrefix(value string, prefixes []string) bool { + value = normalizeName(value) + for _, prefix := range prefixes { + if strings.HasPrefix(value, prefix) { + return true + } + } + return false +} + +func normalizeName(value string) string { + return strings.ToLower(value) +} + +func normalizeIdentityReference(value identityReference) identityReference { + return identityReference{ + ResourceGroup: normalizeName(value.ResourceGroup), + Name: normalizeName(value.Name), + } +} + +func differenceNamedStrings(left, right map[string]string) []string { + result := make([]string, 0) + for normalized, value := range left { + if _, found := right[normalized]; !found { + result = append(result, value) + } + } + sort.Strings(result) + return result +} + +func differenceIdentities(left, right map[identityReference]identityReference) []identityReference { + result := make([]identityReference, 0) + for normalized, value := range left { + if _, found := right[normalized]; !found { + result = append(result, value) + } + } + sortIdentityReferences(result) + return result +} + +func differenceIdentitiesForExistingGroups(left, right map[identityReference]identityReference, existingGroups map[string]string) []identityReference { + result := make([]identityReference, 0) + for normalized, value := range left { + if _, found := existingGroups[normalized.ResourceGroup]; !found { + continue + } + if _, found := right[normalized]; !found { + result = append(result, value) + } + } + sortIdentityReferences(result) + return result +} + +func sortIdentityReferences(values []identityReference) { + sort.Slice(values, func(i, j int) bool { + if values[i].ResourceGroup == values[j].ResourceGroup { + return values[i].Name < values[j].Name + } + return values[i].ResourceGroup < values[j].ResourceGroup + }) +} diff --git a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go new file mode 100644 index 00000000000..b0aefd9ee1a --- /dev/null +++ b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go @@ -0,0 +1,140 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package identitypool + +import ( + "bytes" + "context" + "strings" + "testing" + + "github.com/Azure/ARO-HCP/test/cmd/aro-hcp-tests/slot-manager/slots" + "github.com/Azure/ARO-HCP/test/util/framework" +) + +func TestCompareIdentityPoolInventory(t *testing.T) { + t.Parallel() + + pools := []identityPool{{ + SubscriptionName: "dev-sub", + IdentityContainerPrefix: "aro-hcp-msi-container-dev", + Slots: []slots.ExpandedSlot{{ + IdentityContainerPrefix: "aro-hcp-msi-container-dev-00", + IdentityContainerCount: 2, + }}, + }} + + identityNames := framework.NewDefaultIdentities().ToSlice() + actualIdentities := append([]string{}, identityNames[:len(identityNames)-1]...) + actualIdentities = append(actualIdentities, "unexpected") + actual := subscriptionInventory{ + ResourceGroups: []string{ + "aro-hcp-msi-container-dev-00-00", + "aro-hcp-msi-container-dev-01-00", + "unrelated", + }, + Identities: map[string][]string{ + "aro-hcp-msi-container-dev-00-00": actualIdentities, + }, + } + + result := compareIdentityPoolInventory(pools, actual) + if result.valid() { + t.Fatal("expected drift to be detected") + } + if result.ExpectedResourceGroups != 2 || result.ActualResourceGroups != 2 { + t.Fatalf("unexpected resource group counts: %+v", result) + } + if len(result.MissingResourceGroups) != 1 || result.MissingResourceGroups[0] != "aro-hcp-msi-container-dev-00-01" { + t.Fatalf("unexpected missing resource groups: %v", result.MissingResourceGroups) + } + if len(result.UnexpectedResourceGroups) != 1 || result.UnexpectedResourceGroups[0] != "aro-hcp-msi-container-dev-01-00" { + t.Fatalf("unexpected resource groups: %v", result.UnexpectedResourceGroups) + } + if len(result.MissingIdentities) != 1 || result.MissingIdentities[0].Name != framework.ServiceManagedIdentityName { + t.Fatalf("unexpected missing identities: %v", result.MissingIdentities) + } + if len(result.UnexpectedIdentities) != 1 || result.UnexpectedIdentities[0].Name != "unexpected" { + t.Fatalf("unexpected identities: %v", result.UnexpectedIdentities) + } +} + +func TestValidateOptionsRun(t *testing.T) { + t.Parallel() + + pool := identityPool{ + SubscriptionName: "dev-sub", + SubscriptionID: "sub-id", + IdentityContainerPrefix: "aro-hcp-msi-container-dev", + Slots: []slots.ExpandedSlot{{ + IdentityContainerPrefix: "aro-hcp-msi-container-dev-00", + IdentityContainerCount: 1, + }}, + } + resourceGroup := pool.Slots[0].IdentityContainerNames()[0] + + t.Run("valid", func(t *testing.T) { + t.Parallel() + + var output bytes.Buffer + opts := &ValidateOptions{completedValidateOptions: &completedValidateOptions{ + IdentityPools: []identityPool{pool}, + LoadInventory: func(context.Context, string) (subscriptionInventory, error) { + identityNames := framework.NewDefaultIdentities().ToSlice() + for i := range identityNames { + identityNames[i] = strings.ToUpper(identityNames[i]) + } + return subscriptionInventory{ + ResourceGroups: []string{strings.ToUpper(resourceGroup)}, + Identities: map[string][]string{ + strings.ToUpper(resourceGroup): identityNames, + }, + }, nil + }, + Out: &output, + }} + + if err := opts.Run(context.Background()); err != nil { + t.Fatalf("expected validation to succeed: %v", err) + } + if !strings.Contains(output.String(), "result: valid") { + t.Fatalf("expected valid result output, got %q", output.String()) + } + }) + + t.Run("drift", func(t *testing.T) { + t.Parallel() + + var output bytes.Buffer + opts := &ValidateOptions{completedValidateOptions: &completedValidateOptions{ + IdentityPools: []identityPool{pool}, + LoadInventory: func(context.Context, string) (subscriptionInventory, error) { + return subscriptionInventory{}, nil + }, + Out: &output, + }} + + err := opts.Run(context.Background()) + if err == nil { + t.Fatal("expected validation to fail") + } + if !strings.Contains(err.Error(), "identity pool validation failed") { + t.Fatalf("unexpected validation error: %v", err) + } + if !strings.Contains(output.String(), resourceGroup) { + t.Fatalf("expected missing resource group in output, got %q", output.String()) + } + }) +} From fd55f1b194d8fb35229ac1ffe174e1878f3de9c6 Mon Sep 17 00:00:00 2001 From: Rael Garcia Date: Thu, 27 Aug 2026 13:13:10 +0200 Subject: [PATCH 3/4] fix(slot-manager): report unexpected pool identities (AROSLSRE-1896) --- test/cmd/aro-hcp-tests/slot-manager/DESIGN.md | 4 ++-- .../slot-manager/identity-pool/validate.go | 10 ++++------ .../slot-manager/identity-pool/validate_test.go | 16 +++++++++++++++- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md b/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md index f22203ea990..88ed5087ed9 100644 --- a/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md +++ b/test/cmd/aro-hcp-tests/slot-manager/DESIGN.md @@ -222,8 +222,8 @@ unmanaged pool, so only do that when the subscription owner intends to manage that pool with this command. The command applies one subscription-scoped deployment stack per slot. Each -stack creates or updates the slot's resource groups and the 13 well-known user -assigned identities in every group: +stack creates or updates the slot's resource groups and the 13 well-known +user-assigned managed identities in every group: ```text cluster-api-azure diff --git a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate.go b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate.go index ab74fb7ba21..78abaabb5a1 100644 --- a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate.go +++ b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate.go @@ -252,17 +252,15 @@ func compareIdentityPoolInventory(pools []identityPool, actual subscriptionInven normalizedResourceGroup := normalizeName(resourceGroup) actualIdentitiesByResourceGroup[normalizedResourceGroup] = append(actualIdentitiesByResourceGroup[normalizedResourceGroup], identityNames...) } - for normalizedResourceGroup, resourceGroup := range expectedResourceGroups { + for _, resourceGroup := range expectedResourceGroups { for _, identityName := range expectedIdentityNames { reference := identityReference{ResourceGroup: resourceGroup, Name: identityName} expectedIdentities[normalizeIdentityReference(reference)] = reference } - actualResourceGroup, found := actualResourceGroups[normalizedResourceGroup] - if !found { - continue - } + } + for normalizedResourceGroup, resourceGroup := range actualResourceGroups { for _, identityName := range actualIdentitiesByResourceGroup[normalizedResourceGroup] { - reference := identityReference{ResourceGroup: actualResourceGroup, Name: identityName} + reference := identityReference{ResourceGroup: resourceGroup, Name: identityName} actualIdentities[normalizeIdentityReference(reference)] = reference } } diff --git a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go index b0aefd9ee1a..c3c60acca99 100644 --- a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go +++ b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go @@ -47,6 +47,9 @@ func TestCompareIdentityPoolInventory(t *testing.T) { }, Identities: map[string][]string{ "aro-hcp-msi-container-dev-00-00": actualIdentities, + "aro-hcp-msi-container-dev-01-00": { + framework.ClusterApiAzureMiName, + }, }, } @@ -57,6 +60,9 @@ func TestCompareIdentityPoolInventory(t *testing.T) { if result.ExpectedResourceGroups != 2 || result.ActualResourceGroups != 2 { t.Fatalf("unexpected resource group counts: %+v", result) } + if result.ExpectedIdentities != 26 || result.ActualIdentities != 14 { + t.Fatalf("unexpected identity counts: %+v", result) + } if len(result.MissingResourceGroups) != 1 || result.MissingResourceGroups[0] != "aro-hcp-msi-container-dev-00-01" { t.Fatalf("unexpected missing resource groups: %v", result.MissingResourceGroups) } @@ -66,7 +72,15 @@ func TestCompareIdentityPoolInventory(t *testing.T) { if len(result.MissingIdentities) != 1 || result.MissingIdentities[0].Name != framework.ServiceManagedIdentityName { t.Fatalf("unexpected missing identities: %v", result.MissingIdentities) } - if len(result.UnexpectedIdentities) != 1 || result.UnexpectedIdentities[0].Name != "unexpected" { + if len(result.UnexpectedIdentities) != 2 || + result.UnexpectedIdentities[0] != (identityReference{ + ResourceGroup: "aro-hcp-msi-container-dev-00-00", + Name: "unexpected", + }) || + result.UnexpectedIdentities[1] != (identityReference{ + ResourceGroup: "aro-hcp-msi-container-dev-01-00", + Name: framework.ClusterApiAzureMiName, + }) { t.Fatalf("unexpected identities: %v", result.UnexpectedIdentities) } } From 4641042d65ffd10bc805cb6005aff4dc6cea45b5 Mon Sep 17 00:00:00 2001 From: Rael Garcia Date: Thu, 27 Aug 2026 13:37:50 +0200 Subject: [PATCH 4/4] test(slot-manager): derive identity inventory counts (AROSLSRE-1896) --- .../aro-hcp-tests/slot-manager/identity-pool/validate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go index c3c60acca99..b192dc29106 100644 --- a/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go +++ b/test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go @@ -60,7 +60,7 @@ func TestCompareIdentityPoolInventory(t *testing.T) { if result.ExpectedResourceGroups != 2 || result.ActualResourceGroups != 2 { t.Fatalf("unexpected resource group counts: %+v", result) } - if result.ExpectedIdentities != 26 || result.ActualIdentities != 14 { + if result.ExpectedIdentities != len(identityNames)*2 || result.ActualIdentities != len(actualIdentities)+1 { t.Fatalf("unexpected identity counts: %+v", result) } if len(result.MissingResourceGroups) != 1 || result.MissingResourceGroups[0] != "aro-hcp-msi-container-dev-00-01" {