alan-turing-institute / data-safe-haven

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

Improve credential logging and choice #2064

Closed jemrobinson closed 1 month ago

jemrobinson commented 1 month ago

:white_check_mark: Checklist

:vertical_traffic_light: Depends on

n/a

:arrow_heading_up: Summary

Currently credential confirmation is written to the logs in unnecessary places (e.g. in the Pulumi output) and the user is not given a chance to check that the credentials are correct.

This adds:

:closed_umbrella: Related issues

n/a

:microscope: Tests

Tested on a fresh SRE deployment and on update

github-actions[bot] commented 1 month ago

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  data_safe_haven/external/api
  azure_sdk.py
  credentials.py
Project Total  

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

JimMadge commented 1 month ago

We recently removed the prompt for users to confirm the credentials, was that because it was difficult to keep in restructuring or because we decided it wasn't a good idea?

craddm commented 1 month ago

Was maybe because it was super annoying to have to reconfirm every time? Doing it the first time makes sense

jemrobinson commented 1 month ago

Because it was prompting during non-interactive sessions (i.e. during Pulumi calls) which caused lock-ups. With this PR we can set the skip_confirmation flag in the Pulumi code which should mean that the prompt (and associated messages) only happen in the interactive part of the code.