-
Notifications
You must be signed in to change notification settings - Fork 6
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for picking this up 👍
i am just thinking, they will always now be in the git commit history. so, anyone can go back and look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment that I want to discuss. rest LGTM.
Client ID and tenant ID are not secrets per se. They are discoverable knowing domain name to anyone on the internet I believe. @liammoat correct me if I'm wrong please. |
Yeah - no major concerns from me with these being in the history. |
Unfortunately, the template tests do not succeed with placeholder values. We should investigate further why. I will abandon this PR and raise a work item separately. |
Replaces the following hardcoded vaules in
docker/.env.template
with placeholders:AZURE_CLIENT_ID
,AZURE_CLIENT_SECRET
,AZURE_TENANT_ID
,DataHubFhirServer__TemplateImage
Tests
Documentation