alan-turing-institute / data-safe-haven

https://data-safe-haven.readthedocs.io
BSD 3-Clause "New" or "Revised" License
61 stars 15 forks source link

Miscellaneous fixes from TRESA testing #2068

Closed jemrobinson closed 3 months ago

jemrobinson commented 3 months ago

:white_check_mark: Checklist

:vertical_traffic_light: Depends on

n/a

:arrow_heading_up: Summary

Miscellaneous fixes from TRESA testing. Mostly documentation, but the following code changes:

:closed_umbrella: Related issues

Closes #2063 Closes #2066

:microscope: Tests

Tested through TRESA deployment

:thought_balloon: Thoughts

github-actions[bot] commented 3 months ago

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  data_safe_haven/external/api
  graph_api.py 440
  data_safe_haven/infrastructure/programs
  declarative_sre.py
  data_safe_haven/validators
  validators.py
Project Total  

This report was generated by python-coverage-comment-action

jemrobinson commented 3 months ago

Thoughts on the hard-coded list of allowed locations @JimMadge @craddm ?

JimMadge commented 3 months ago

Thoughts on the hard-coded list of allowed locations @JimMadge @craddm ?

I almost commented on that. I think it is a sensible compromise.

We could use az CLI to get the list, but that would require signing in, be difficult to test in CI, and slow down the code.

craddm commented 3 months ago

Thoughts on the hard-coded list of allowed locations @JimMadge @craddm ?

I almost commented on that. I think it is a sensible compromise.

We could use az CLI to get the list, but that would require signing in, be difficult to test in CI, and slow down the code.

Same, I had a look if you could get a list using the Python SDK and couldn't see an obvious one, and only one mentioned on stackoverflow that would require logging in

jemrobinson commented 3 months ago

We have a function for this in our AzureSDK here: https://github.com/alan-turing-institute/data-safe-haven/blob/78ba7b77270ddb680203f818874fc1c5ceb092ee/data_safe_haven/external/api/azure_sdk.py#L677-L696

However, this means you'd need to log in to the Azure CLI before being able to validate location and I wasn't sure whether this was a good idea or not.