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

feat: add unix sockets support for URLs #874

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

nlachfr
Copy link

@nlachfr nlachfr commented Nov 4, 2021

Porposal for adding unix sockets support for URLs

Related issue(s)

#871

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security. vulnerability, I
    confirm that I got green light (please contact
    security@ory.sh) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further Comments

@nlachfr nlachfr requested a review from aeneasr as a code owner November 4, 2021 21:48
@CLAassistant
Copy link

CLAassistant commented Nov 4, 2021

CLA assistant check
All committers have signed the CLA.

@nlachfr nlachfr closed this Nov 4, 2021
@nlachfr nlachfr changed the title Add the use of unix sockets for URLs Add unix sockets support for URLs Nov 4, 2021
@nlachfr nlachfr reopened this Nov 4, 2021
@codecov
Copy link

codecov bot commented Nov 4, 2021

Codecov Report

Merging #874 (a557489) into master (737320f) will increase coverage by 0.11%.
Report is 346 commits behind head on master.
The diff coverage is 72.16%.

@@            Coverage Diff             @@
##           master     #874      +/-   ##
==========================================
+ Coverage   62.47%   62.59%   +0.11%     
==========================================
  Files         102      103       +1     
  Lines        4813     4895      +82     
==========================================
+ Hits         3007     3064      +57     
- Misses       1531     1548      +17     
- Partials      275      283       +8     
Files Changed Coverage Δ
proxy/proxy.go 67.00% <44.44%> (-3.22%) ⬇️
pipeline/authn/authenticator_cookie_session.go 72.44% <57.69%> (-7.56%) ⬇️
helper/transport.go 78.84% <78.84%> (ø)
pipeline/authn/authenticator_bearer_token.go 69.23% <100.00%> (+4.01%) ⬆️
pipeline/authz/remote.go 80.00% <100.00%> (ø)
pipeline/authz/remote_json.go 96.29% <100.00%> (ø)
rule/validator.go 83.92% <100.00%> (+3.57%) ⬆️

@nlachfr nlachfr marked this pull request as draft November 4, 2021 21:57
@nlachfr nlachfr changed the title Add unix sockets support for URLs feat: add unix sockets support for URLs Nov 4, 2021
@aeneasr
Copy link
Member

aeneasr commented Nov 22, 2021

Thank you, this looks great! It looks like the CLA bot is not properly detecting your signature. To fix this, try the following:

$ git commit  --amend --author="Author Name <email@address.com>"

Ensure that Author Name is replaced with your GitHub username (e.g. aeneasr) and that the email address is replaced with the email address you have set up in GitHub (e.g. 3372410+aeneasr@users.noreply.github.com):

$ git commit  --amend --author="aeneasr <3372410+aeneasr@users.noreply.github.com>"

Once that is done, you can force-push your changes (make sure to push to the correct remote and branch!):

$ git push --force <remote> HEAD:<branch>

@nlachfr nlachfr force-pushed the feat/unix-socket-support-for-upstreams branch from a7827ee to 3c8c779 Compare November 22, 2021 19:45
@nlachfr nlachfr force-pushed the feat/unix-socket-support-for-upstreams branch from 3c8c779 to d057b58 Compare November 22, 2021 19:54
@nlachfr
Copy link
Author

nlachfr commented Nov 22, 2021

I updated my commits signature and agreed to the CLA

Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Thank you! Could you please include tests and documentation to show case how this feature is configured?

@nlachfr
Copy link
Author

nlachfr commented Dec 2, 2021

Regarding documentation, do you think I should add a specific section for unix sockets URIs ?
For now, I have added a small description on fields able to handle it.

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