Skip to content

Commit

Permalink
Addresses PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
wbreza committed Nov 7, 2022
1 parent e99011d commit 524aa99
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 15 deletions.
10 changes: 10 additions & 0 deletions cli/azd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
// Importing for infrastructure provider plugin registrations
_ "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning/bicep"
_ "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning/terraform"
"github.com/azure/azure-dev/cli/azd/pkg/input"

"github.com/azure/azure-dev/cli/azd/internal"
"github.com/azure/azure-dev/cli/azd/internal/telemetry"
Expand Down Expand Up @@ -152,6 +153,15 @@ func BuildCmd[F any](
return err
}

console := input.GetConsole(ctx)
_, err = console.Confirm(ctx, input.ConsoleOptions{
Message: "Debugger attached?",
DefaultValue: true,
})
if err != nil {
return err
}

return action.Run(ctx)
}

Expand Down
5 changes: 4 additions & 1 deletion cli/azd/pkg/commands/pipeline/azdo_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,10 @@ func (p *AzdoCiProvider) configureConnection(
console input.Console) error {

if authType == AuthTypeOidc {
return fmt.Errorf("Azure DevOps does not support OIDC authentication. Service Principal with client ID and client secret must be used. %w", ErrAuthNotSupported)
return fmt.Errorf(
"Azure DevOps does not support OIDC authentication. Service Principal with client ID and client secret must be used. %w",
ErrAuthNotSupported,
)
}

azureCredentials, err := parseCredentials(ctx, credentials)
Expand Down
6 changes: 5 additions & 1 deletion cli/azd/pkg/commands/pipeline/github_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,13 +371,17 @@ func (p *GitHubCiProvider) configureConnection(
if infraOptions.Provider == provisioning.Terraform {
// Throw error if Oidc is explicitly requested
if authType == AuthTypeOidc {
return fmt.Errorf("Terraform does not support OIDC authentication. Service Principal with client ID and client secret must be used. %w", ErrAuthNotSupported)
return fmt.Errorf(
"Terraform does not support OIDC authentication. Service Principal with client ID and client secret must be used. %w",
ErrAuthNotSupported,
)
}

// If not explicitly set, show warning
console.Message(
ctx,
output.WithWarningFormat(
//nolint:lll
"WARNING: Terraform provisioning does not support OIDC authentication, defaulting to Service Principal with client ID and client secret.\n",
),
)
Expand Down
79 changes: 73 additions & 6 deletions cli/azd/pkg/graphsdk/fic_request_builders_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,13 @@ func TestGetFederatedCredentialById(t *testing.T) {
expected := federatedCredentials[0]

mockContext := mocks.NewMockContext(context.Background())
graphsdk_mocks.RegisterFederatedCredentialGetItemMock(mockContext, *application.Id, *expected.Id, http.StatusOK, &expected)
graphsdk_mocks.RegisterFederatedCredentialGetItemMock(
mockContext,
*application.Id,
*expected.Id,
http.StatusOK,
&expected,
)

client, err := graphsdk_mocks.CreateGraphClient(mockContext)
require.NoError(t, err)
Expand All @@ -101,7 +107,13 @@ func TestGetFederatedCredentialById(t *testing.T) {

t.Run("Error", func(t *testing.T) {
mockContext := mocks.NewMockContext(context.Background())
graphsdk_mocks.RegisterFederatedCredentialGetItemMock(mockContext, *application.Id, "bad-id", http.StatusNotFound, nil)
graphsdk_mocks.RegisterFederatedCredentialGetItemMock(
mockContext,
*application.Id,
"bad-id",
http.StatusNotFound,
nil,
)

client, err := graphsdk_mocks.CreateGraphClient(mockContext)
require.NoError(t, err)
Expand All @@ -126,7 +138,9 @@ func TestCreateFederatedCredential(t *testing.T) {
client, err := graphsdk_mocks.CreateGraphClient(mockContext)
require.NoError(t, err)

actual, err := client.ApplicationById(*application.Id).FederatedIdentityCredentials().Post(*mockContext.Context, &expected)
actual, err := client.ApplicationById(*application.Id).
FederatedIdentityCredentials().
Post(*mockContext.Context, &expected)
require.NoError(t, err)
require.NotNil(t, actual)
require.Equal(t, *expected.Id, *actual.Id)
Expand All @@ -151,13 +165,61 @@ func TestCreateFederatedCredential(t *testing.T) {
})
}

func TestPatchFederatedCredential(t *testing.T) {
expected := federatedCredentials[0]

t.Run("Success", func(t *testing.T) {
mockContext := mocks.NewMockContext(context.Background())
graphsdk_mocks.RegisterFederatedCredentialPatchItemMock(
mockContext,
*application.Id,
*expected.Id,
http.StatusNoContent,
)

client, err := graphsdk_mocks.CreateGraphClient(mockContext)
require.NoError(t, err)

err = client.
ApplicationById(*application.Id).
FederatedIdentityCredentialById(*expected.Id).
Update(*mockContext.Context, &expected)

require.NoError(t, err)
})

t.Run("Error", func(t *testing.T) {
mockContext := mocks.NewMockContext(context.Background())
graphsdk_mocks.RegisterFederatedCredentialPatchItemMock(
mockContext,
*application.Id,
*expected.Id,
http.StatusBadRequest,
)

client, err := graphsdk_mocks.CreateGraphClient(mockContext)
require.NoError(t, err)

err = client.
ApplicationById(*application.Id).
FederatedIdentityCredentialById(*expected.Id).
Update(*mockContext.Context, &graphsdk.FederatedIdentityCredential{})

require.Error(t, err)
})
}

func TestDeleteFederatedCredential(t *testing.T) {
credentialId := "credential-to-delete"

t.Run("Success", func(t *testing.T) {

mockContext := mocks.NewMockContext(context.Background())
graphsdk_mocks.RegisterFederatedCredentialDeleteItemMock(mockContext, *application.Id, credentialId, http.StatusNoContent)
graphsdk_mocks.RegisterFederatedCredentialDeleteItemMock(
mockContext,
*application.Id,
credentialId,
http.StatusNoContent,
)

client, err := graphsdk_mocks.CreateGraphClient(mockContext)
require.NoError(t, err)
Expand All @@ -172,7 +234,12 @@ func TestDeleteFederatedCredential(t *testing.T) {

t.Run("Error", func(t *testing.T) {
mockContext := mocks.NewMockContext(context.Background())
graphsdk_mocks.RegisterFederatedCredentialDeleteItemMock(mockContext, *application.Id, credentialId, http.StatusNotFound)
graphsdk_mocks.RegisterFederatedCredentialDeleteItemMock(
mockContext,
*application.Id,
credentialId,
http.StatusNotFound,
)

client, err := graphsdk_mocks.CreateGraphClient(mockContext)
require.NoError(t, err)
Expand Down
52 changes: 45 additions & 7 deletions cli/azd/test/mocks/graphsdk/mocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ func RegisterApplicationDeleteItemMock(
statusCode int,
) {
mockContext.HttpClient.When(func(request *http.Request) bool {
return request.Method == http.MethodDelete && strings.Contains(request.URL.Path, fmt.Sprintf("/applications/%s", appId))
return request.Method == http.MethodDelete &&
strings.Contains(request.URL.Path, fmt.Sprintf("/applications/%s", appId))
}).RespondFn(func(request *http.Request) (*http.Response, error) {
return mocks.CreateEmptyHttpResponse(request, statusCode)
})
Expand Down Expand Up @@ -245,9 +246,15 @@ func RegisterRoleAssignmentPutMock(mockContext *mocks.MockContext, statusCode in
})
}

func RegisterFederatedCredentialsListMock(mockContext *mocks.MockContext, applicationId string, statusCode int, federatedCredentials []graphsdk.FederatedIdentityCredential) {
func RegisterFederatedCredentialsListMock(
mockContext *mocks.MockContext,
applicationId string,
statusCode int,
federatedCredentials []graphsdk.FederatedIdentityCredential,
) {
mockContext.HttpClient.When(func(request *http.Request) bool {
return request.Method == http.MethodGet && strings.Contains(request.URL.Path, fmt.Sprintf("/applications/%s/federatedIdentityCredentials", applicationId))
return request.Method == http.MethodGet &&
strings.Contains(request.URL.Path, fmt.Sprintf("/applications/%s/federatedIdentityCredentials", applicationId))
}).RespondFn(func(request *http.Request) (*http.Response, error) {
listResponse := graphsdk.FederatedIdentityCredentialListResponse{
Value: federatedCredentials,
Expand All @@ -261,9 +268,15 @@ func RegisterFederatedCredentialsListMock(mockContext *mocks.MockContext, applic
})
}

func RegisterFederatedCredentialCreateItemMock(mockContext *mocks.MockContext, applicationId string, statusCode int, federatedCredential *graphsdk.FederatedIdentityCredential) {
func RegisterFederatedCredentialCreateItemMock(
mockContext *mocks.MockContext,
applicationId string,
statusCode int,
federatedCredential *graphsdk.FederatedIdentityCredential,
) {
mockContext.HttpClient.When(func(request *http.Request) bool {
return request.Method == http.MethodPost && strings.Contains(request.URL.Path, fmt.Sprintf("/applications/%s", applicationId))
return request.Method == http.MethodPost &&
strings.Contains(request.URL.Path, fmt.Sprintf("/applications/%s", applicationId))
}).RespondFn(func(request *http.Request) (*http.Response, error) {
if federatedCredential == nil {
return mocks.CreateEmptyHttpResponse(request, statusCode)
Expand All @@ -273,6 +286,23 @@ func RegisterFederatedCredentialCreateItemMock(mockContext *mocks.MockContext, a
})
}

func RegisterFederatedCredentialPatchItemMock(
mockContext *mocks.MockContext,
applicationId string,
credentialId string,
statusCode int,
) {
mockContext.HttpClient.When(func(request *http.Request) bool {
return request.Method == http.MethodPatch &&
strings.Contains(
request.URL.Path,
fmt.Sprintf("/applications/%s/federatedIdentityCredentials/%s", applicationId, credentialId),
)
}).RespondFn(func(request *http.Request) (*http.Response, error) {
return mocks.CreateEmptyHttpResponse(request, statusCode)
})
}

func RegisterFederatedCredentialGetItemMock(
mockContext *mocks.MockContext,
appId string,
Expand All @@ -281,7 +311,11 @@ func RegisterFederatedCredentialGetItemMock(
federatedCredential *graphsdk.FederatedIdentityCredential,
) {
mockContext.HttpClient.When(func(request *http.Request) bool {
return request.Method == http.MethodGet && strings.Contains(request.URL.Path, fmt.Sprintf("/applications/%s/federatedIdentityCredentials/%s", appId, federatedCredentialId))
return request.Method == http.MethodGet &&
strings.Contains(
request.URL.Path,
fmt.Sprintf("/applications/%s/federatedIdentityCredentials/%s", appId, federatedCredentialId),
)
}).RespondFn(func(request *http.Request) (*http.Response, error) {
if federatedCredential == nil {
return mocks.CreateEmptyHttpResponse(request, statusCode)
Expand All @@ -298,7 +332,11 @@ func RegisterFederatedCredentialDeleteItemMock(
statusCode int,
) {
mockContext.HttpClient.When(func(request *http.Request) bool {
return request.Method == http.MethodDelete && strings.Contains(request.URL.Path, fmt.Sprintf("/applications/%s/federatedIdentityCredentials/%s", appId, federatedCredentialId))
return request.Method == http.MethodDelete &&
strings.Contains(
request.URL.Path,
fmt.Sprintf("/applications/%s/federatedIdentityCredentials/%s", appId, federatedCredentialId),
)
}).RespondFn(func(request *http.Request) (*http.Response, error) {
return mocks.CreateEmptyHttpResponse(request, statusCode)
})
Expand Down

0 comments on commit 524aa99

Please sign in to comment.