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.

Sign up for GitHub

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

Jump to bottom

Centralize one source for configuration #414

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Centralize one source for configuration #414

wants to merge 25 commits into from

Conversation

kdhttps
Copy link
Contributor

@kdhttps kdhttps commented Jan 20, 2022 •
edited
Loading

close #382 task-2

  • rate-limit config
  • session config
  • proxy config

@christian-hawk first two are done but still need your to view on this #382 (comment)

@kdhttps
test(rate-limiter.test.js): test rate limiter
e6c1f3b
@kdhttps
test(app-factory.test.js): test rate limiter middleware
64e854e
@kdhttps
refactor(rate-limiter.js): remove env config
f33c83e
@kdhttps
refactor(app-factory.js): configure rate-limit middleware
39cfcdf
@kdhttps
refactor(app.js): configure rate-limit middleware
9597cac
@kdhttps
test(rate-limiter.test.js): test rate limit reconfig
89e559f
@kdhttps
test(app.init.spec.js): check rateLimit config
bd2b2ee
@kdhttps
feat(rate-limiter.js): handle the reconfig rate limit
3e7b647
@kdhttps
refactor(helper.js): refactor test as per new rate limit config
8c6d454
@kdhttps
test(test.js): add session config
4ffebd4
@kdhttps
test(app-factory.test.js): test empty session middleware
1e34dd7
@kdhttps
test(session.test.js): the module should init express session
b4342fe
@kdhttps
refactor(session.spec.js): use latest config for session test
cf1cb49
@kdhttps
feat(session.js): configure session with dynamic values
f67b48b
@kdhttps
feat(app.js): config session in app
7746e61
@kdhttps
refactor(rate-limiter.js): refactor code
f59d816
@kdhttps
refactor(app.js): remove commented code
2a69ced
@kdhttps
fix(app-factory.js): fix session middleware problem
bfff9ba
@kdhttps
test(helper.js): fix bdd test
b1bef92
@kdhttps
Merge branch 'master' of github.com:GluuFederation/gluu-passport into…
fe32f68 
… 382_task_2
@codecov
Copy link

codecov bot commented Jan 20, 2022 •
edited
Loading

Codecov Report

Merging #414 (31572ef) into master (09221af) will increase coverage by 0.61%.
The diff coverage is 100.00%.

❗ Current head 31572ef differs from pull request most recent head 4f0fc9e. Consider uploading reports for the commit 4f0fc9e to get more accurate results

@@            Coverage Diff             @@
##           master     #414      +/-   ##
==========================================
+ Coverage   78.58%   79.20%   +0.61%     
==========================================
  Files          36       36              
  Lines         780      803      +23     
==========================================
+ Hits          613      636      +23     
  Misses        167      167
Impacted Files Coverage Δ
config/test.js 100.00% <ø> (ø)
server/app-factory.js 94.28% <100.00%> (+0.16%) ⬆️
server/app.js 96.00% <100.00%> (+0.87%) ⬆️
server/utils/http-global-proxy.js 100.00% <100.00%> (ø)
server/utils/rate-limiter.js 100.00% <100.00%> (ø)
server/utils/session.js 100.00% <100.00%> (ø)

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 09221af...4f0fc9e. Read the comment docs.

@sonarcloud
Copy link

sonarcloud bot commented Jan 20, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@kdhttps kdhttps changed the title Centralize configuration Centralize one source for configuration Jan 20, 2022
@kdhttps kdhttps self-assigned this Jan 20, 2022
@christian-hawk
Copy link
Contributor

close #382 task-2

  • rate-limit config
  • session config
  • proxy config

@christian-hawk first two are done but still need your to view on this #382 (comment)

I agree with you #382 (comment)

So proxy config is not done yet? If is not done please change PR to draft and request review when ready. Thanks mate :)

@kdhttps
Copy link
Contributor Author

kdhttps commented Mar 21, 2022

So proxy config is not done yet? If is not done please change PR to draft and request review when ready. Thanks mate :)

not done yet mate, sure I will do, thank you :)

@kdhttps kdhttps removed the request for review from christian-hawk March 21, 2022 05:34
@kdhttps kdhttps marked this pull request as draft March 21, 2022 05:36
@sonarcloud
Copy link

sonarcloud bot commented Mar 21, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@kdhttps kdhttps marked this pull request as ready for review March 22, 2022 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christian-hawk christian-hawk Awaiting requested review from christian-hawk

Assignees

@kdhttps kdhttps

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rate-limiting settings being loaded from node-config instead of application
2 participants
@kdhttps @christian-hawk