Skip to content
This repository was archived by the owner on Apr 13, 2020. It is now read-only.

[FEATURE] scaffold app and helm repo in spk setup command #398

Merged
merged 19 commits into from
Mar 18, 2020
Merged

Conversation

dennisseah
Copy link
Collaborator

@andrebriggs
Copy link
Collaborator

@dennisseah looks good. We might want to look at #402 first

@dennisseah
Copy link
Collaborator Author

@dennisseah looks good. We might want to look at #402 first

Thanks for helping @andrebriggs issue1113x is based on issue1113. That's issue1113x has more changes (to create the lifecycle and build pipelines that branch issue1113 does not have). I cannot create PR for issue1113x branch until this one is merge, otherwise the PR will be huge. Thanks

@@ -150,6 +176,7 @@ export const prompt = async (): Promise<RequestContext> => {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Line 163 or prompt.ts
message: Do you like create a sample application repository?, should be
message: Would you like to create a sample application repository?,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

accept

@@ -76,6 +86,9 @@ describe("test prompt function", () => {
az_sp_password: "a510c1ff-358c-4ed4-96c8-eb23f42bbc5b",
az_sp_tenant: "72f988bf-86f1-41af-91ab-2d7cd011db47"
});
jest.spyOn(inquirer, "prompt").mockResolvedValueOnce({
acr_name: "testACR"
});
jest.spyOn(subscriptionService, "getSubscriptions").mockResolvedValueOnce([
Copy link
Collaborator

Choose a reason for hiding this comment

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

In one case, I attempt to create a service principal with the az command line after logging in. I received an error getting a token request due to an unauthorized client. This may have occurred because I may have sent your authentication request to the wrong tenant. In any case, we should add an additional safety net to select the appropriate subscription ID after login to create the service principal in.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

can you please elaborate on this? Do you want to select subscription Id first and then create service principal? Currently, we are doing the other way around, we can service principal and automatically figure out subscription id

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here's the error I'm getting:
After az login the service principal gets created under the wrong tenant I believe when I select to generate a SP

error: Error: Get Token request returned http error: 400 and server response: {"error":"unauthorized_client","error_description":"AADSTS700016: Application with identifier '#' was not found in the directory '#'. This can happen if the application has not been installed by the administrator of the tenant or consented to by any user in the tenant. You may have sent your authentication request to the wrong tenant.\r\nTrace ID:#\r\nCorrelation ID: #r\nTimestamp: 2020-03-17 18:35:28Z","error_codes":[700016],"timestamp":"2020-03-17 18:35:28Z","trace_id":"#","correlation_id":"#","error_uri":"https://login.microsoftonline.com/error?code=700016"}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@andrebriggs how can this happen. We automatically create a service principal (which has Contributor role). How can service principal being created in the wrong tenant?

Copy link
Collaborator

@andrebriggs andrebriggs Mar 18, 2020

Choose a reason for hiding this comment

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

@NathanielRose let's save this behavior for another PR. Good catch though

@dennisseah dennisseah merged commit 7b212c9 into master Mar 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create app repo in SPK setup command
3 participants