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

Changed Admin app to single tenant #715

Merged
merged 17 commits into from
Aug 14, 2024

Conversation

neelavarshad
Copy link
Contributor

Updated admin app configs
Changed variable name for checking if app regs are provided

@santhoshb-msft
Copy link
Contributor

santhoshb-msft commented Jun 8, 2024

Hi @neelavarshad lets talk more on Monday before we add more or merge this.

@neelavarshad neelavarshad requested a review from bkhabazan June 10, 2024 20:57
@neelavarshad
Copy link
Contributor Author

Fixes comments from #698
Fixes #716 to explicitly make fulfillment app reg single tenant

@neelavarshad neelavarshad marked this pull request as ready for review June 10, 2024 21:09
Copy link
Contributor

@bkhabazan bkhabazan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test complete, all changes are approved.

@neelavarshad neelavarshad requested a review from msalemcode June 19, 2024 18:35
@neelavarshad neelavarshad requested a review from bkhabazan June 21, 2024 00:27
@bkhabazan
Copy link
Contributor

All tests passed and approved

@neelavarshad
Copy link
Contributor Author

@santhoshb-msft I pushed the default set to false. You can validate on this now.

@santhoshb-msft
Copy link
Contributor

Hi @neelavarshad I tried to install this branch twice but keeps failing , can you please rerun on your side again

image

@neelavarshad
Copy link
Contributor Author

neelavarshad commented Jul 22, 2024

@bkhabazan Did you see anything like what Santhosh is encountering?

@santhoshb-msft I just ran the deployment on my single-tenant-admin branch and it ran through ok
I did get a warning about upcoming changes, but no errors related to dotnet ef

image

@bkhabazan
Copy link
Contributor

bkhabazan commented Jul 23, 2024

@neelavarshad I did not see any error messages when running the code. I was able to deploy successfully.

@santhoshb-msft
Copy link
Contributor

I just ran again and same issue, lets connect internally to if the script is missing anything

@neelavarshad
Copy link
Contributor Author

@santhoshb-msft Please try again with the latest changes I added. Thanks!

@santhoshb-msft
Copy link
Contributor

Hi @neelavarshad just checking if the multiple url display issue is fixed?

@neelavarshad
Copy link
Contributor Author

@santhoshb-msft This is fixed now

@santhoshb-msft santhoshb-msft self-assigned this Aug 8, 2024
@santhoshb-msft
Copy link
Contributor

santhoshb-msft commented Aug 13, 2024

@neelavarshad have you seen the below by any chance?

image

@neelavarshad
Copy link
Contributor Author

I ran through several tests last week and didn't see anything with MSI. What point of the script is this coming up on? Is it only on my branch you're seeing this?

@santhoshb-msft
Copy link
Contributor

santhoshb-msft commented Aug 13, 2024

I ran through several tests last week and didn't see anything with MSI. What point of the script is this coming up on? Is it only on my branch you're seeing this?

Yes @neelavarshad, just running your branch, just before finish
git clone https://github.com/neelavarshad/Microsoft-commercial-marketplace-transactable-SaaS-offer-SDK.git -b single-tenant-admin; `
Can you run like on a clean az cloud shell?

@neelavarshad
Copy link
Contributor Author

@santhoshb-msft
I just ran on a clean cloudshell and didn't see what you're seeing. It ran the update firewall commands and directly went to cleanup without any errors. Can you share the full error via DM? I'll look into it if you'd like.
image

@santhoshb-msft
Copy link
Contributor

@santhoshb-msft I just ran on a clean cloudshell and didn't see what you're seeing. It ran the update firewall commands and directly went to cleanup without any errors. Can you share the full error via DM? I'll look into it if you'd like. image

Sure, checking again and will reach out internally

Copy link
Contributor

@santhoshb-msft santhoshb-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Tested with new install
  2. Validate all three-app registration and their redirect uris
  3. Validated app service setting for both admin portal and customer portal
  4. Switched single to muliti tenant . for future we might need docs on how to swtich, 3 steps, enble app service flag, switch app reg and add user to knownusers

@santhoshb-msft santhoshb-msft merged commit 5433873 into Azure:main Aug 14, 2024
7 checks passed
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.

3 participants