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

Add Lua endpoint to support dynamic certificate serving functionality #2889

Merged
merged 1 commit into from
Aug 13, 2018

Conversation

hnrytrn
Copy link
Contributor

@hnrytrn hnrytrn commented Aug 2, 2018

What this PR does / why we need it:

This PR adds a Lua endpoint to store and retrieve certificates for hosts in a shared dictionary. This is needed to support the functionality of serving SSL certificates dynamically instead of reloading NGINX.

There will be two other related PRs following this: one to post the certificates and skip the NGINX reloads on the controller side, and the other to serve the certificates stored in the shared dictionary

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 2, 2018
@hnrytrn hnrytrn changed the title Dynamic cert endpoint Add Lua endpoint to support dynamic certificate serving functionality Aug 2, 2018
@codecov-io
Copy link

codecov-io commented Aug 2, 2018

Codecov Report

Merging #2889 into master will decrease coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2889      +/-   ##
==========================================
- Coverage   47.65%   47.57%   -0.08%     
==========================================
  Files          76       76              
  Lines        5500     5484      -16     
==========================================
- Hits         2621     2609      -12     
+ Misses       2541     2540       -1     
+ Partials      338      335       -3
Impacted Files Coverage Δ
internal/ingress/controller/template/template.go 66.32% <100%> (+0.07%) ⬆️
internal/net/net.go 72.72% <0%> (-2.28%) ⬇️
internal/ingress/controller/nginx.go 11.28% <0%> (-0.4%) ⬇️
internal/ingress/controller/config/config.go 98.26% <0%> (-0.03%) ⬇️
internal/ingress/metric/collectors/socket.go 79.81% <0%> (+0.53%) ⬆️

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 c97a90f...5200a38. Read the comment docs.

@aledbf
Copy link
Member

aledbf commented Aug 3, 2018

@hnrytrn please squash the commits

@hnrytrn hnrytrn force-pushed the dynamic-cert-endpoint branch 2 times, most recently from 350eb21 to d9ba68a Compare August 3, 2018 14:06
@ElvinEfendi
Copy link
Member

/assign ElvinEfendi

end

local err_buf = {}
-- Update certificates and private keys for each host
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this comment adds any value. If you think it's hard to follow the logic below then I'd suggest you extract it into a separate function with descriptive name.

@yuyang0 yuyang0 mentioned this pull request Aug 6, 2018
@ElvinEfendi
Copy link
Member

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 7, 2018
@ElvinEfendi
Copy link
Member

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 7, 2018
@ElvinEfendi
Copy link
Member

I added hold because I don't want this to end up in 0.18.0 release.

@ElvinEfendi
Copy link
Member

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ElvinEfendi, hnrytrn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ElvinEfendi
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 13, 2018
@k8s-ci-robot k8s-ci-robot merged commit 3f5af6e into kubernetes:master Aug 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants