Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions python/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# ARO-RP/python
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the readme!

We also have this one: https://github.com/Azure/ARO-RP/blob/master/docs/az-aro-python-development.md should we consolidate them?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should. I remembered that az-aro-python-development.md existed after I started this one. I have changes in my working tree to consolidate them.


[draft]

Welcome to the ARO command-line extension.

Code in these directories make up the commands and logic for the `az aro` CLI.

## Structure

### `python/az/aro/azext_aro`

This is where the majority of human written code lives.

highlights:

- `__init__.py` - Azure CLI entrypoint
- `commands.py` - ARO extension command structure definitions
- `custom.py` - Logic and helper methods for subcommands
- `_help.py` - Help output definitions

### `python/az/aro/build`

Locally generated code

### `python/az/aro/azext_aro/aaz`

Generated code vendored from AZ tooling (upstream?) that we occasionally change
when we need new classes or functions.

### `python/az/client`

More vendored code

## Prerequisites

Ensure you have `python` installed. I recommend using `asdf`
([ref](https://asdf-vm.com/)) if you aren't already.

## Setup

From the repo root, run `make pyenv` to create and setup our local python
virtual environment.
6 changes: 6 additions & 0 deletions python/az/aro/azext_aro/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,9 @@
short-summary: Get required identities
long-summary: Get required identities for creating a cluster with managed identities.
"""

helps['aro identity create-required'] = """
type: command
short-summary: Create required identities
long-summary: Create required identities to prepare for creating a cluster with managed identities.
"""
1 change: 1 addition & 0 deletions python/az/aro/azext_aro/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,5 @@ def load_command_table(loader, _):
g.custom_command('validate', 'aro_validate')

with loader.command_group('aro identity', aro_sdk, client_factory=cf_aro) as g:
g.custom_command('create-required', 'aro_identity_create_required', supports_no_wait=False)
g.custom_command('get-required', 'aro_identity_get_required', supports_no_wait=False)
121 changes: 116 additions & 5 deletions python/az/aro/azext_aro/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import os
from base64 import b64decode
import textwrap
import uuid

import azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2025_07_25.models as openshiftcluster

Expand Down Expand Up @@ -40,8 +41,10 @@
)
from azext_aro._validators import validate_subnets
from azext_aro._dynamic_validators import validate_cluster_create, validate_cluster_delete
from azext_aro.aaz.latest.identity import Create as identity_create
from azext_aro.aaz.latest.identity import Delete as identity_delete
from azext_aro.aaz.latest.network.vnet.subnet import Show as subnet_show
from azext_aro.aaz.latest.role.assignment import Create as roleassignment_create

from knack.log import get_logger

Expand Down Expand Up @@ -767,10 +770,6 @@ def aro_identity_get_required(*,
worker_subnet,
vnet,
vnet_resource_group_name=None) -> None:

if not vnet_resource_group_name:
vnet_resource_group_name = resource_group_name

if version not in aro_get_versions(client, location):
raise ValidationError("--version invalid")

Expand All @@ -782,11 +781,19 @@ def aro_identity_get_required(*,
if not role_set:
raise RuntimeError("Could not find identity requirements for provided version and location.")

# NOTE: i (adprice) don't like that we're resorting to `logger.warning`
# here rather than basic `print()`. it looks weird seeing a wall of yellow
# text.
#
# Can we return a list[str] instead? i'll experiment with how the printer
# displays that.

logger.warning("Use the following commands to create the required managed identities:")
print_identity_create_cmd(resource_group_name, 'aro-cluster', location)
for role in role_set.platform_workload_identity_roles:
print_identity_create_cmd(resource_group_name, role.operator_name, location)

sub_id = get_subscription_id(cmd.cli_ctx)
auth_client = get_mgmt_service_client(cmd.cli_ctx, ResourceType.MGMT_AUTHORIZATION)
logger.warning("\nUse the following commands to create the required role assignments"
" over virtual network and/or subnets:")
Expand All @@ -804,7 +811,9 @@ def aro_identity_get_required(*,
for scope in scopes:
print_role_assignment_create_cmd(
f"$(az identity show -g '{resource_group_name}' -n '{role.operator_name}' --query principalId -o tsv)",
role.role_definition_id,
# NOTE: i don't know why, but role.role_definition_id is not
# the full resource ID
f"{resource_id(subscription=sub_id)}{role.role_definition_id}",
scope
)

Expand All @@ -825,6 +834,108 @@ def aro_identity_get_required(*,
)


def aro_identity_create_required(*,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this function is duplicated from our previous command get-required. Can we move the duplicated code to a separate function where we can call it both here and in get-required?

cmd,
client,
resource_group_name,
location,
version,
master_subnet,
worker_subnet,
vnet,
vnet_resource_group_name=None) -> list:

# FIXME: figure out how to do the fancy "in progress" spinner
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, can we have the CLI print what it's doing during each creation? Something like Creating identity "cloud-controller-manager" in resource group "caden-miwi" for version "4.19.20" and etc...


created_identities = list()
created_roleassignments = list()

if version not in aro_get_versions(client, location):
raise ValidationError("--version invalid")

role_set = None
for _set in client.platform_workload_identity_role_sets.list(location):
if version.startswith(_set.open_shift_version):
role_set = _set

if not role_set:
raise RuntimeError("Could not find identity requirements for provided version and location.")

idcreate = identity_create(cli_ctx=cmd.cli_ctx)
racreate = roleassignment_create(cli_ctx=cmd.cli_ctx)

# cluster top-level identity
cluster_id = idcreate(command_args={
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we execute any creation, should we prompt the user Y/n like we do on cluster deletion? (I don't have a strong opinion since we don't do it on cluster create, but it might be nice here)

Required identities and role assignments for version "4.19.20" in resource group "caden-miwi" will be created. Proceed? (Y/n)

"location": location,
"resource_group": resource_group_name,
"resource_name": "aro-cluster",
})
created_identities.append(cluster_id)

auth_client = get_mgmt_service_client(cmd.cli_ctx, ResourceType.MGMT_AUTHORIZATION)
for role in role_set.platform_workload_identity_roles:
# cluster identities
identity = idcreate(command_args={
"location": location,
"resource_group": resource_group_name,
"resource_name": role.operator_name,
})
created_identities.append(identity)

definition = auth_client.role_definitions.get_by_id(role.role_definition_id)
scopes: list[str]
for permissions in definition.permissions:
for action in permissions.actions:
if action.startswith("Microsoft.Network/virtualNetworks/subnets/"):
scopes = [master_subnet, worker_subnet]
elif action.startswith("Microsoft.Network/virtualNetworks/"):
scopes = [vnet]
break

# vnet/subnet role assignments
for scope in scopes:
ra = racreate(command_args={
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we can't directly modify the aaz itself, we'll need some kind of "check if this already exists" behavior implemented here before we execute the create so that we can run this command idempotently. In its current state, if I delete a previously created identity and re-run this command, I will get an "already exists" error because there's a leftover orphaned role assignment:

(RoleAssignmentExists) The role assignment already exists.
Code: RoleAssignmentExists
Message: The role assignment already exists.

Fixing this will account for cases where a role assignment or identity failed to create and the customer needs to re-run. It's a bit tedious removing role assignments manually and most customers won't have the option to remove and re-create the whole resource group every time.

"principal_id": identity["principalId"],
"principal_type": "ServicePrincipal",
"role_definition_id": role.role_definition_id,
"scope": scope,
"role_assignment_name": str(uuid.uuid4()),
})
created_roleassignments.append(ra)

# platform workload identity role assignment
topra = racreate(command_args={
"principal_id": cluster_id["principalId"],
"principal_type": "ServicePrincipal",
"role_definition_id": resource_id(
subscription=get_subscription_id(cmd.cli_ctx),
namespace="Microsoft.Authorization",
type="roleDefinitions",
name=ARO_FEDERATED_CREDENTIAL_ROLE
),
"scope": identity["id"],
"role_assignment_name": str(uuid.uuid4()),
})
created_roleassignments.append(topra)

aad = AADManager(cmd.cli_ctx)
spra = racreate(command_args={
"principal_id": aad.get_service_principal_id(FP_CLIENT_ID),
"principal_type": "ServicePrincipal",
"role_definition_id": resource_id(
subscription=get_subscription_id(cmd.cli_ctx),
namespace="Microsoft.Authorization",
type="roleDefinitions",
name=FP_SERVICE_PRINCIPAL_ROLE,
),
"scope": vnet,
"role_assignment_name": str(uuid.uuid4())
})
created_roleassignments.append(spra)

return created_identities + created_roleassignments


def ensure_resource_permissions(cli_ctx, oc, fail, sp_obj_ids):
try:
# Get cluster resources we need to assign permissions on, sort to ensure the same order of operations
Expand Down
Loading