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

Use auth-helper to auth webapp&functions. #843

Merged
merged 9 commits into from
Oct 9, 2019
Merged

Use auth-helper to auth webapp&functions. #843

merged 9 commits into from
Oct 9, 2019

Conversation

andxu
Copy link
Contributor

@andxu andxu commented Sep 30, 2019

Use auth-helper to auth webapp&functions.

@codecov-io
Copy link

codecov-io commented Sep 30, 2019

Codecov Report

Merging #843 into develop will decrease coverage by 0.53%.
The diff coverage is 3.7%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #843      +/-   ##
=============================================
- Coverage      45.08%   44.55%   -0.54%     
+ Complexity       768      765       -3     
=============================================
  Files            133      134       +1     
  Lines           5119     5169      +50     
  Branches         729      742      +13     
=============================================
- Hits            2308     2303       -5     
- Misses          2640     2692      +52     
- Partials         171      174       +3
Impacted Files Coverage Δ Complexity Δ
...a/com/microsoft/azure/maven/spring/DeployMojo.java 5.42% <ø> (ø) 4 <0> (ø) ⬇️
...microsoft/azure/maven/common/utils/MavenUtils.java 26.66% <ø> (ø) 3 <0> (?)
...a/com/microsoft/azure/maven/spring/ConfigMojo.java 77.24% <ø> (ø) 43 <0> (ø) ⬇️
...crosoft/azure/maven/spring/AbstractSpringMojo.java 46.07% <ø> (ø) 14 <0> (ø) ⬇️
...microsoft/azure/maven/auth/AzureClientFactory.java 0% <0%> (ø) 0 <0> (?)
...a/com/microsoft/azure/maven/AbstractAzureMojo.java 46.62% <2.94%> (-11.39%) 0 <0> (ø)
...rosoft/azure/maven/auth/AzureAuthHelperLegacy.java 56.25% <50%> (ø) 0 <0> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a683be...22955a0. Read the comment docs.

@@ -0,0 +1,43 @@
<project xmlns="http://maven.apache.org/POM/4.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

Why we move the resources to spring-cloud-maven-common ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines +298 to +313
if (this.settings.getServer(auth.getServerId()) != null) {
try {
auth = MavenSettingHelper.getAuthConfigurationFromServer(session, settingsDecrypter, auth.getServerId());
} catch (MavenDecryptException e) {
throw new AzureAuthFailureException(e.getMessage());
}
} else {
throw new AzureAuthFailureException(String.format("Unable to get server('%s') from settings.xml.", auth.getServerId()));
}
}

final List<ConfigurationProblem> problems = auth.validate();
if (problems.stream().anyMatch(problem -> problem.getSeverity() == Severity.ERROR)) {
throw new AzureAuthFailureException(String.format("Unable to validate auth configuration due to the following errors: %s",
problems.stream().map(ConfigurationProblem::getErrorMessage).collect(Collectors.joining("\n"))));
}
Copy link
Member

@Flanker32 Flanker32 Oct 8, 2019

Choose a reason for hiding this comment

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

Shall we move the check of server id as well as the validation to MavenSettingHelper.getAuthConfigurationFromServer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be done later

@andxu andxu merged commit 01cd468 into develop Oct 9, 2019
@andxu andxu deleted the andy-auth2 branch October 9, 2019 04:48
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.

None yet

3 participants