User username as VM username rather than random ID #4333

marrobi · 2025-02-06T16:12:27Z

Users email prior to the @ sign (as external users have unhelpful UPNs), or if no email UPN prior to the @ sign, as the username, limits to 20 chars.

Needed to update resource processor to pick OwnerId from the message and used AD TF provider to get details from user objectID.

Fixes microsoft#4325 Add platform-specific initialization scripts for dev container. * **Add `initialize.sh` script**: Create directories `.azure` and `.config` in the user's home directory using Bash. * **Add `initialize.cmd` script**: Create directories `.azure` and `.config` in the user's profile directory using Windows command line. * **Update `initializeCommand` in `devcontainer.json`**: Use array syntax to reference the new platform-specific initialization scripts. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/microsoft/AzureTRE/issues/4325?shareId=XXXX-XXXX-XXXX-XXXX).

…robi/username-as-username

github-actions · 2025-02-06T16:14:23Z

Unit Test Results

0 tests 0 ✅ 0s ⏱️
0 suites 0 💤
0 files 0 ❌

Results for commit 580e43c.

♻️ This comment has been updated with latest results.

Copilot reviewed 4 out of 20 changed files in this pull request and generated no comments.

Files not reviewed (16)

templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/template_schema.json: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/.terraform.lock.hcl: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/data.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/linuxvm.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/locals.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/main.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/outputs.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/variables.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/template_schema.json: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/.terraform.lock.hcl: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/data.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/locals.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/main.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/outputs.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/variables.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/windowsvm.tf: Language not supported

Comments suppressed due to low confidence (1)

resource_processor/resources/commands.py:145

The comment contains a spelling mistake: 'ot' should be 'to', 'detaisl' should be 'details', and 'objec tID' should be 'object ID'.

return msg_body.get("ownerId")  # not included in all messages

Copilot reviewed 4 out of 20 changed files in this pull request and generated no comments.

Files not reviewed (16)

templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/template_schema.json: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/.terraform.lock.hcl: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/data.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/linuxvm.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/locals.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/main.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/outputs.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/variables.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/template_schema.json: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/.terraform.lock.hcl: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/data.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/locals.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/main.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/outputs.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/variables.tf: Language not supported
templates/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/windowsvm.tf: Language not supported

Comments suppressed due to low confidence (1)

resource_processor/resources/commands.py:145

[nitpick] The parameter name 'owner_id' should be checked for consistency with the rest of the codebase.

return msg_body.get("ownerId")

marrobi · 2025-02-06T16:18:32Z

/test-extended

github-actions · 2025-02-06T16:18:47Z

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/13183175950 (with refid def25300)

(in response to this comment from @marrobi)

jonnyry · 2025-02-13T20:57:01Z

@marrobi ok to update this branch? will give it a test locally

…robi/username-as-username

marrobi · 2025-02-13T21:58:43Z

@marrobi ok to update this branch? will give it a test locally

Done.

jonnyry · 2025-02-14T11:54:21Z

@marrobi ok to update this branch? will give it a test locally

Done.

Gave it a quick test on a local instance and worked nicely.

jonnyry

LGTM. Couple of small comments re username calculation.

jonnyry · 2025-02-14T12:09:09Z

@marrobi Re the username calculation logic -

 admin_username = (
    length(data.azuread_user.user.mail) > 0 ?
    substr(element(split("@", data.azuread_user.user.mail), 0), 0, 20) :
    substr(
      contains(element(split("@", data.azuread_user.user.user_principal_name), 0), "#EXT#") ?
      element(split("#EXT#", element(split("@", data.azuread_user.user.user_principal_name), 0)), 0) :
      element(split("@", data.azuread_user.user.user_principal_name), 0),
      0, 20
    )
  )

We use internal member accounts, and so would prefer if the UPN was used first - so the username aligns with the user's Entra username, rather than external email accounts which will come from a variety of domains and potentially in different formats.

Though understand a lot of people will be using external guest accounts.

Could the logic be something like this instead, so the UPN is preferred unless its a guest account:

if ((UPN contains "#EXT#") and (email is not blank)) then
  use email address
else
  use UPN

I think that would look something like:

 admin_username = (
    ((length(data.azuread_user.user.mail) > 0) && (contains(data.azuread_user.user.user_principal_name, "#EXT#"))) ?
    substr(element(split("@", data.azuread_user.user.mail), 0), 0, 20) :
    substr(
      contains(element(split("@", data.azuread_user.user.user_principal_name), 0), "#EXT#") ?
      element(split("#EXT#", element(split("@", data.azuread_user.user.user_principal_name), 0)), 0) :
      element(split("@", data.azuread_user.user.user_principal_name), 0),
      0, 20
    )
  )

...ates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/locals.tf

...es/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/locals.tf

...ates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/locals.tf

marrobi · 2025-02-17T13:11:04Z

@jonnyry I've updated as per your PR comments, I've also added a fix for the VM deletion issue #4135

Are you able to give this another test? I don't have an environment deployed at the moment.

…robi/username-as-username

...workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/windowsvm.tf

...es/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/locals.tf

marrobi · 2025-02-19T15:33:34Z

/test

github-actions · 2025-02-19T15:33:52Z

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13416348783 (with refid def25300)

(in response to this comment from @marrobi)

marrobi added 6 commits February 6, 2025 09:23

WIP

a68e577

Merge branch 'main' of https://github.com/microsoft/AzureTRE into mar…

8c494eb

…robi/username-as-username

Works with Linux and Windows

fbdee79

Merge branch 'main' of https://github.com/microsoft/AzureTRE into mar…

2aff78b

…robi/username-as-username

Revert changes

8900234

restore files

ce54f37

marrobi requested a review from Copilot February 6, 2025 16:15

Copilot AI reviewed Feb 6, 2025

View reviewed changes

fix linting

c99c11a

marrobi requested a review from Copilot February 6, 2025 16:17

Copilot AI reviewed Feb 6, 2025

View reviewed changes

marrobi marked this pull request as ready for review February 6, 2025 16:18

marrobi requested review from jonnyry, tamirkamara and LizaShak February 6, 2025 16:18

Changelog

2d1c400

Merge branch 'main' of https://github.com/microsoft/AzureTRE into mar…

ff6fae2

…robi/username-as-username

jonnyry approved these changes Feb 14, 2025

View reviewed changes

jonnyry reviewed Feb 14, 2025

View reviewed changes

...ates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/locals.tf Outdated Show resolved Hide resolved

jonnyry reviewed Feb 14, 2025

View reviewed changes

...es/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/locals.tf Outdated Show resolved Hide resolved

jonnyry reviewed Feb 14, 2025

View reviewed changes

...es/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/locals.tf Show resolved Hide resolved

jonnyry reviewed Feb 14, 2025

View reviewed changes

...ates/workspace_services/guacamole/user_resources/guacamole-azure-linuxvm/terraform/locals.tf Outdated Show resolved Hide resolved

marrobi added 3 commits February 17, 2025 12:55

Merge branch 'main' into marrobi/username-as-username

ae3ad49

Address PR comments.

5f8f953

add deletion fix.

db9c04e

marrobi added 4 commits February 18, 2025 12:16

Merge branch 'main' of https://github.com/microsoft/AzureTRE into mar…

34dace7

…robi/username-as-username

Fix username generation.

0192362

Fix strcontains

2a93c65

Optimise username calculation

b5e9cb9

marrobi requested a review from yuvalyaron February 18, 2025 18:59

yuvalyaron requested changes Feb 19, 2025

View reviewed changes

...workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/windowsvm.tf Show resolved Hide resolved

...es/workspace_services/guacamole/user_resources/guacamole-azure-windowsvm/terraform/locals.tf Show resolved Hide resolved

yuvalyaron approved these changes Feb 19, 2025

View reviewed changes

Merge branch 'main' into marrobi/username-as-username

580e43c

marrobi enabled auto-merge (squash) February 19, 2025 15:33

marrobi merged commit 5c7d725 into microsoft:main Feb 19, 2025
12 checks passed

marrobi deleted the marrobi/username-as-username branch February 19, 2025 16:41

This was referenced Feb 19, 2025

Random username generator for Linux VMs should not generate digits as the first character #3693

Closed

Unable to delete a Guacamole User Resource if it is in the 'stopped' state #4135

Closed

jonnyry mentioned this pull request Feb 21, 2025

Set stairlockp Airlock Processor storage account firewall to "Enabled from selected virtual networks and IP addresses" #4388

Closed

marrobi mentioned this pull request Feb 21, 2025

E2E Extended tests failing as service principal does not exist as a user in AD #4396

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User username as VM username rather than random ID #4333

User username as VM username rather than random ID #4333

marrobi commented Feb 6, 2025 •

edited

Loading

github-actions bot commented Feb 6, 2025 •

edited

Loading

marrobi commented Feb 6, 2025

github-actions bot commented Feb 6, 2025

jonnyry commented Feb 13, 2025

marrobi commented Feb 13, 2025

jonnyry commented Feb 14, 2025

jonnyry left a comment •

edited

Loading

jonnyry commented Feb 14, 2025

marrobi commented Feb 17, 2025

marrobi commented Feb 19, 2025

github-actions bot commented Feb 19, 2025

User username as VM username rather than random ID #4333

User username as VM username rather than random ID #4333

Conversation

marrobi commented Feb 6, 2025 • edited Loading

github-actions bot commented Feb 6, 2025 • edited Loading

Unit Test Results

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marrobi commented Feb 6, 2025

github-actions bot commented Feb 6, 2025

jonnyry commented Feb 13, 2025

marrobi commented Feb 13, 2025

jonnyry commented Feb 14, 2025

jonnyry left a comment • edited Loading

Choose a reason for hiding this comment

jonnyry commented Feb 14, 2025

marrobi commented Feb 17, 2025

marrobi commented Feb 19, 2025

github-actions bot commented Feb 19, 2025

marrobi commented Feb 6, 2025 •

edited

Loading

github-actions bot commented Feb 6, 2025 •

edited

Loading

jonnyry left a comment •

edited

Loading