Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Move to using api:// for AAD Application "identifier uris" #1243

Merged
merged 5 commits into from
Sep 17, 2021

Conversation

stishkin
Copy link
Contributor

@stishkin stishkin commented Sep 13, 2021

Summary of the Pull Request

What is this about?

PR Checklist

  • Applies to work item: App registration fails when deploying new instance of OneFuzz #1248
  • CLA signed. If not, go over here and sign the CLI.
  • Tests added/passed
  • Requires documentation to be updated
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Info on Pull Request

What does this include?
Updating Application ID URI in AppRegistration to a URI accepted by Azure. Moving URI from https:// to api://.
Updating all the code that requires Application ID URI to use a new format.

Validation Steps Performed

How does someone test & validate?
Run integration test, more tests will be done by @chkeita

src/cli/onefuzz/backend.py Outdated Show resolved Hide resolved
src/cli/onefuzz/backend.py Outdated Show resolved Hide resolved
@stishkin stishkin force-pushed the stas/test branch 6 times, most recently from d033c75 to e65a7e8 Compare September 15, 2021 17:31
@stishkin stishkin requested a review from chkeita September 15, 2021 19:25
src/deployment/deploy.py Outdated Show resolved Hide resolved
src/deployment/deploy.py Outdated Show resolved Hide resolved
@stishkin stishkin force-pushed the stas/test branch 2 times, most recently from c71e8ca to cfb306d Compare September 15, 2021 19:33
src/deployment/deploy.py Outdated Show resolved Hide resolved
@stishkin stishkin force-pushed the stas/test branch 2 times, most recently from 03d9eb9 to afaba3c Compare September 15, 2021 20:08
src/deployment/deploy.py Outdated Show resolved Hide resolved
@stishkin stishkin force-pushed the stas/test branch 2 times, most recently from 5cc75b1 to 297cd75 Compare September 15, 2021 21:29
@stishkin stishkin marked this pull request as ready for review September 15, 2021 22:11
@stishkin stishkin linked an issue Sep 15, 2021 that may be closed by this pull request
@bmc-msft
Copy link
Contributor

Can this PR have a different name than test?

@stishkin stishkin changed the title test App registration fails when deploying new instance of OneFuzz Sep 15, 2021
@nharper285
Copy link
Contributor

nharper285 commented Sep 15, 2021

Received an error while trying to deploy to multitenant:

INFO:deploy:checking if RBAC already exists
INFO:deploy:creating Application registration
INFO:deploy:creating service principal
Traceback (most recent call last):
  File "deploy.py", line 1109, in <module>
    main()
  File "deploy.py", line 1103, in main
    state[1](client)
  File "deploy.py", line 394, in setup_rbac
    if not app.identifier_uris.contains(api_url):
AttributeError: 'list' object has no attribute 'contains'

I used the following deploy command:
python deploy.py westus2 noharper-mulit noharper-multi "<email>" --multi_tenant_domain "<domain>"

src/deployment/deploy.py Outdated Show resolved Hide resolved
src/deployment/deploy.py Outdated Show resolved Hide resolved
@chkeita
Copy link
Contributor

chkeita commented Sep 16, 2021

Validation in progress

  • old cli instance can connect to new
  • multi-tenant deployment functional

@nharper285
Copy link
Contributor

I tested and can confirm we have a running job in multi-tenant instance.

@bmc-msft bmc-msft requested a review from chkeita September 17, 2021 16:36
@bmc-msft bmc-msft changed the title App registration fails when deploying new instance of OneFuzz Move to using api:// for AAD Application "identifier uris" Sep 17, 2021
@bmc-msft bmc-msft merged commit 2e267a8 into microsoft:main Sep 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App registration fails when deploying new instance of OneFuzz
4 participants