Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge SHM and context resource deployment #1963

Conversation

jemrobinson
Copy link
Member

@jemrobinson jemrobinson commented Jun 25, 2024

✅ Checklist

  • You have given your pull request a meaningful title (e.g. Enable foobar integration rather than 515 foobar).
  • You are targeting the appropriate branch. If you're not certain which one this is, it should be develop.
  • Your branch is up-to-date with the target branch (it probably was when you started, but it may have changed since then).

🚦 Depends on

n/a

⤴️ Summary

This PR moves SHM deployment out of Pulumi and into AzureSDK commands. It also refactors the CLI interface so only the following commands are needed to deploy and SHM and SREs

$ dsh context add --admin-group-name $GROUP_NAME --description $HUMAN_FRIENDLY_NAME --name $SAFE_NAME --subscription $SUBSCRIPTION_NAME
$ dsh shm deploy --entra-tenant-id $TENANT_ID  --fqdn $FQDN  --location $LOCATION
$ dsh config template --file $YAML_FILE_PATH
$ dsh config upload $YAML_FILE_PATH
$ dsh sre deploy $SRE_NAME

It moves some config values between Context and SHMConfig so that fewer things depend on the (local-only) Context and more things use the (shared) SHMConfig. The things remaining in Context are all human-readable (no GUIDs)

Config examples

$ dsh context show
Current context: contoso
        Admin group name: Safe Haven Test Admins
        Description: DSG 2024 - Contoso
        Name: contoso
        Subscription name: Data Safe Haven Development
$ dsh config show-shm
azure:
  location: uksouth
  subscription_id: <removed GUID>
  tenant_id: <removed GUID>
shm:
  admin_group_id: <removed GUID>
  entra_tenant_id: <removed GUID>
  fqdn: green.develop.turingsafehaven.ac.uk
$ dsh config show apple
azure:
  location: uksouth
  subscription_id: <removed GUID>
  tenant_id: <removed GUID>
description: Apple Project
name: apple
sre:
  admin_email_address: safehavendevs@turing.ac.uk
  admin_ip_addresses:
  - <removed IP address>
  - <removed IP address>
  - <removed IP address>
  - <removed IP address>
  data_provider_ip_addresses:
  - <removed IP address>
  - <removed IP address>
  - <removed IP address>
  databases:
  - postgresql
  remote_desktop:
    allow_copy: true
    allow_paste: true
  research_user_ip_addresses:
  - <removed IP address>
  - <removed IP address>
  - <removed IP address>
  software_packages: pre-approved
  timezone: Europe/London
  workspace_skus:
  - Standard_D4s_v5

🌂 Related issues

Closes #1850
Closes #1896
Closes #1898
Closes #1900
Closes #1962
Closes #1965

🔬 Tests

Tested locally

@jemrobinson jemrobinson requested review from a team as code owners June 25, 2024 12:15
@jemrobinson jemrobinson requested review from craddm and JimMadge June 25, 2024 12:15
Copy link
Member

@JimMadge JimMadge left a comment

Choose a reason for hiding this comment

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

Looks good.

I have some questions, and there are a few pieces I'm not very keen on.

I think there is scope to improve the coverage in data_safe_haven/commands/ and data_safe_haven/config/.

data_safe_haven/config/shm_config.py Outdated Show resolved Hide resolved
data_safe_haven/config/shm_config.py Outdated Show resolved Hide resolved
data_safe_haven/infrastructure/programs/imperative_shm.py Outdated Show resolved Hide resolved
data_safe_haven/infrastructure/programs/imperative_shm.py Outdated Show resolved Hide resolved
data_safe_haven/config/context_manager.py Outdated Show resolved Hide resolved
data_safe_haven/external/api/azure_cli.py Show resolved Hide resolved
data_safe_haven/commands/shm.py Show resolved Hide resolved
jemrobinson and others added 2 commits June 25, 2024 15:28
Co-authored-by: Jim Madge <jim+github@jmadge.com>
Co-authored-by: Jim Madge <jim+github@jmadge.com>
@jemrobinson jemrobinson force-pushed the 1900-merge-shm-with-context branch from 0e71103 to 1d0524c Compare June 25, 2024 18:59
@JimMadge
Copy link
Member

@craddm Would appreciate your thoughts on the conversations here.

@jemrobinson jemrobinson force-pushed the 1900-merge-shm-with-context branch from b4d1796 to c12091d Compare June 27, 2024 12:09
@jemrobinson
Copy link
Member Author

@JimMadge : Anything else you want to see here?

@jemrobinson jemrobinson force-pushed the 1900-merge-shm-with-context branch from 62e6884 to 7d359a0 Compare June 27, 2024 13:29
@jemrobinson jemrobinson merged commit 012279e into alan-turing-institute:develop Jun 27, 2024
11 checks passed
@jemrobinson jemrobinson deleted the 1900-merge-shm-with-context branch January 30, 2025 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants