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

Remove ACTIONS_ACR & consolidate secrets #2654

Merged
merged 3 commits into from
Sep 28, 2022

Conversation

tamirkamara
Copy link
Collaborator

@tamirkamara tamirkamara commented Sep 28, 2022

What is being addressed

  1. Github Actions currently use a separate ACR (aka ACTIONS_ACR) to store the devcontainer images. This requires more initial setup which confuses users.
  2. We ask users to provide both AZURE_CREDENTIALS and ARM_* style secrets which are duplication

We can make this simpler...
Closes microsoft/AzureTRE-Deployment#10

Migration

  1. Validate your Github environment has the AZURE_CREDENTIALS secret defined, in the following format:
{
  "clientId": "xxx",
  "clientSecret": "xxx",
  "tenantId": "xxx",
  "subscriptionId": "xxx",
"resourceManagerEndpointUrl": "management.azure.com"
}
  1. Delete the following Github Secrets: ACTIONS_ACR_NAME, ACTIONS_ACR_PASSWORD, ARM_CLIENT_ID, ARM_CLIENT_SECRET, ARM_SUBSCRIPTION_ID, ARM_TENANT_ID

How is this addressed

  • TRE devcontainer will be stored in the ACR defined for the environment in the ACR_NAME secret
  • Change the actions so that they will "wait" for the ACR to be created for a new environment without failure. This makes it transparent for the user without any special setup required.
  • Actions access the ACR via azure login (az acr login)
  • Remove all ARM_* style credential secrets and fall back to using AZURE_CREDENTIALS

@github-actions
Copy link

github-actions bot commented Sep 28, 2022

Unit Test Results

17 tests  +12   17 ✔️ +13   6s ⏱️ - 2h 24m 39s
  1 suites ±  0     0 💤 ±  0 
  1 files   ±  0     0  -   1 

Results for commit f7d3769. ± Comparison against base commit b89a336.

This pull request removes 5 and adds 17 tests. Note that renamed tests count towards both.
test_airlock ‑ test_airlock_import_flow
test_shared_services ‑ test_create_shared_service[tre-shared-service-gitea]
test_shared_services ‑ test_patch_firewall
test_workspace_services ‑ test_create_guacamole_service_into_aad_workspace
test_workspace_services ‑ test_create_guacamole_service_into_base_workspace
test_provisioned_health_api ‑ test_health
test_shared_service_templates ‑ test_get_shared_service_template[tre-shared-service-firewall]
test_shared_service_templates ‑ test_get_shared_service_template[tre-shared-service-gitea]
test_shared_service_templates ‑ test_get_shared_service_templates[tre-shared-service-firewall]
test_shared_service_templates ‑ test_get_shared_service_templates[tre-shared-service-gitea]
test_ui ‑ test_ui
test_workspace_service_templates ‑ test_create_workspace_service_templates
test_workspace_service_templates ‑ test_get_workspace_service_template[tre-service-azureml]
test_workspace_service_templates ‑ test_get_workspace_service_template[tre-service-guacamole]
test_workspace_service_templates ‑ test_get_workspace_service_template[tre-service-innereye]
…

♻️ This comment has been updated with latest results.

@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:29 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:36 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 05:39 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 06:08 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:28 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:30 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:30 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:30 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:54 Inactive
@tamirkamara tamirkamara temporarily deployed to CICD September 28, 2022 07:56 Inactive
@tamirkamara tamirkamara enabled auto-merge (squash) September 28, 2022 08:39
@tamirkamara tamirkamara merged commit bf071cd into main Sep 28, 2022
@tamirkamara tamirkamara deleted the tamirkamara/remove-actions-acr branch September 28, 2022 09:18
@marrobi
Copy link
Member

marrobi commented Oct 11, 2022

@tamirkamara @LizaShak the docs https://microsoft.github.io/AzureTRE/tre-admins/environment-variables/ stull reference ARM_ credentials - does this need updating?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Registry Issue when creating registry as part of pre-deployment
3 participants