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

Enable login authentication for eureka #4663

Merged
merged 1 commit into from
Dec 14, 2022

Conversation

nobodyiam
Copy link
Member

What's the purpose of this PR

Enable login authentication for eureka

Brief changelog

  1. add configs for eureka authentication
  2. add documentation on how to enable eureka authentication

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit tests to verify the code.
  • Run mvn clean test to make sure this pull request doesn't break anything.
  • Update the CHANGES log.

@codecov
Copy link

codecov bot commented Nov 26, 2022

Codecov Report

Merging #4663 (10e72ca) into master (8ddbe0a) will decrease coverage by 0.02%.
The diff coverage is 29.41%.

❗ Current head 10e72ca differs from pull request most recent head 6ae389f. Consider uploading reports for the commit 6ae389f to get more accurate results

@@             Coverage Diff              @@
##             master    #4663      +/-   ##
============================================
- Coverage     47.16%   47.13%   -0.03%     
  Complexity     1647     1647              
============================================
  Files           347      347              
  Lines         10643    10660      +17     
  Branches       1057     1060       +3     
============================================
+ Hits           5020     5025       +5     
- Misses         5321     5331      +10     
- Partials        302      304       +2     
Impacted Files Coverage Δ
...nfigservice/ConfigServerEurekaServerConfigure.java 33.33% <29.41%> (-66.67%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Value("${apollo.eureka.server.security.username}")
private String username;
@Value("${apollo.eureka.server.security.password}")
private String password;
Copy link
Contributor

Choose a reason for hiding this comment

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

How about let user can config eureka's username and password in configdb?

  • only config once even when scale configservice

but the password will save in configdb

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a good idea! Storing the username/password in the DB makes it consistent among multiple config services.
The only issue here is apollo.eureka.server.security.enabled, it's hard to read it from DB as it is used in ConditionalOnProperty, which is triggered before the database initialization phase.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Anilople I changed the implementation a little and the apollo.eureka.server.security related items could be configured in the configdb, please help to take a look.

Copy link
Contributor

Choose a reason for hiding this comment

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

How it work?...

Are there some methods read data the from configdb?...

Copy link
Member Author

Choose a reason for hiding this comment

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

The BizConfig would put all the ServerConfig properties to the Spring environment. It won't work for ConditionalOnProperty as it's initiated later than the ConditionalOnProperty processing so I put the if (eurekaSecurityEnabled) check in the method.

Copy link
Contributor

Choose a reason for hiding this comment

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

It works.

When I change ServerConfig in configdb

image

then try to access http://localhost:8080/eureka/apps/APOLLO-CONFIGSERVICE

it need auth as expect.

@nobodyiam nobodyiam force-pushed the eureka-server-security branch 2 times, most recently from 884541c to fd96c72 Compare December 4, 2022 13:18
docs/en/deployment/distributed-deployment-guide.md Outdated Show resolved Hide resolved
docs/en/deployment/distributed-deployment-guide.md Outdated Show resolved Hide resolved
@Value("${apollo.eureka.server.security.username}")
private String username;
@Value("${apollo.eureka.server.security.password}")
private String password;
Copy link
Contributor

Choose a reason for hiding this comment

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

How it work?...

Are there some methods read data the from configdb?...

@nobodyiam nobodyiam force-pushed the eureka-server-security branch from 10e72ca to af7e860 Compare December 12, 2022 00:44
Copy link
Contributor

@Anilople Anilople left a comment

Choose a reason for hiding this comment

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

LGTM

@Value("${apollo.eureka.server.security.username}")
private String username;
@Value("${apollo.eureka.server.security.password}")
private String password;
Copy link
Contributor

Choose a reason for hiding this comment

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

It works.

When I change ServerConfig in configdb

image

then try to access http://localhost:8080/eureka/apps/APOLLO-CONFIGSERVICE

it need auth as expect.

@nobodyiam nobodyiam force-pushed the eureka-server-security branch from af7e860 to 6ae389f Compare December 14, 2022 00:43
@nobodyiam nobodyiam merged commit 7df79bf into apolloconfig:master Dec 14, 2022
@nobodyiam nobodyiam deleted the eureka-server-security branch December 14, 2022 00:47
@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2022
@nobodyiam nobodyiam added this to the 2.1.0 milestone Feb 5, 2023
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.

2 participants