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

Update doc and config schema for tre_id #3541

Merged
merged 7 commits into from
Jul 20, 2023
Merged

Conversation

t-young31
Copy link
Contributor

Resolves #3536

What is being addressed

The documentation and config.yaml schema suggests underscores can be used in a tre_id. This updates the. documentation and schema to only allow lowercase letters. Maybe hyphens are ok? (looks like all storage accounts use a replace on hyphens but not sure if there are other resources that assume the id only has lowercase letters)

@github-actions github-actions bot added the external PR from an external contributor label Jun 6, 2023
@marrobi marrobi requested a review from tamirkamara June 6, 2023 20:03
@marrobi
Copy link
Member

marrobi commented Jun 6, 2023

Thanks @t-young31.

I'm sure we have had this discussion before, agree it is likely to restrict the ID up front. I think the issue is if people already have a tre_id with an underscore how do they proceed. with an upgrade. However given this mongo DB was added a while ago, not sure how many there are, if any.

Let us do a bit of research and see if we have any active deployments with _ in the ID.

cc @itye-msft

@t-young31
Copy link
Contributor Author

I'd vote to reduce the friction for new adopters and anyone with an existing deployment, which I guess would be pre core kv needs to update their config schema

Hopefully the doc updates are useful even if the schema doesn't get updated!

@tamirkamara
Copy link
Collaborator

We use numbers in our TREIDs so that would need to come back in too.

@t-young31
Copy link
Contributor Author

have done

"Use only lowercase letters" from the doc is then incorrect?

@marrobi
Copy link
Member

marrobi commented Jun 22, 2023

We definitely have numbers in TRE IDs, so need to update the docs.

@github-actions
Copy link

github-actions bot commented Jun 22, 2023

Unit Test Results

0 tests   0 ✔️  0s ⏱️
0 suites  0 💤
0 files    0

Results for commit ebcf8e6.

♻️ This comment has been updated with latest results.

@marrobi
Copy link
Member

marrobi commented Jun 22, 2023

@t-young31 actually the doc says "alphanumeric" rather than letters?

@t-young31
Copy link
Contributor Author

"Use only lowercase letters. Choose wisely!"

1. Decide on a name for your `tre_id`, which is an alphanumeric (with underscores allowed) ID for the Azure TRE instance. The value will be used in various Azure resources and AAD application names. It **needs to be globally unique and less than 12 characters in length**. Use only lowercase letters. Choose wisely!

@marrobi
Copy link
Member

marrobi commented Jun 22, 2023

Yup, this needs to be consistent. As far as I am concerned use lowercase alphanumeric characters makes sense.

@t-young31
Copy link
Contributor Author

feel free to do a find & replace alphanumerics -> alphanumeric characters on this branch

@marrobi marrobi added the bug Something isn't working label Jun 27, 2023
@marrobi
Copy link
Member

marrobi commented Jul 20, 2023

/tet-force-approve

@marrobi marrobi enabled auto-merge (squash) July 20, 2023 13:56
@github-actions
Copy link

🤖 pr-bot 🤖

/tet-force-approve is not recognised as a valid command.

You can use the following commands:
    /test - build, deploy and run smoke tests on a PR
    /test-extended - build, deploy and run smoke & extended tests on a PR
    /test-extended-aad - build, deploy and run smoke & extended AAD tests on a PR
    /test-shared-services - test the deployment of shared services on a PR build
    /test-force-approve - force approval of the PR tests (i.e. skip the deployment checks)
    /test-destroy-env - delete the validation environment for a PR (e.g. to enable testing a deployment from a clean start after previous tests)
    /help - show this help

(in response to this comment from @marrobi)

@marrobi
Copy link
Member

marrobi commented Jul 20, 2023

/test-force-approve

@github-actions
Copy link

🤖 pr-bot 🤖

✅ Marking tests as complete (for commit ebcf8e6)

(in response to this comment from @marrobi)

@marrobi marrobi merged commit 1bb54d4 into microsoft:main Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working external PR from an external contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tre_id can't have underscores
3 participants