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

Auto-generate certs in-memory #2795

Merged
merged 1 commit into from
Jan 29, 2018
Merged

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Jan 25, 2018

The dashboard is currently writing autogenerated cert files to a directory managed by the kubelet secret mounter.

At best, those files are auto-removed shortly after being written and the server startup races with the removal.

At worst, the location is mounted readonly and the dashboard fails to start.

There's no reason to write files to a location where they will just be cleaned up later (and doing so is breaking the dashboard when the location is read-only... see https://github.com/kubernetes/kubernetes/pull/58720/files#diff-7ab88bb7654d97946d6328f11f29d177)

This PR changes the dashboard to keep auto-generated certs in memory

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 25, 2018
@codecov
Copy link

codecov bot commented Jan 25, 2018

Codecov Report

Merging #2795 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2795   +/-   ##
=======================================
  Coverage   54.17%   54.17%           
=======================================
  Files         564      564           
  Lines       12199    12199           
=======================================
  Hits         6609     6609           
  Misses       5334     5334           
  Partials      256      256
Impacted Files Coverage Δ
src/app/backend/cert/ecdsa/creator.go 14.89% <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 2c7e17e...29535d3. Read the comment docs.

@floreks
Copy link
Member

floreks commented Jan 26, 2018

/lgtm
/approve

Thanks! This definitely helps, however we have bigger issue here. I thought that if we'd mount secret in RW mode then it would sync secret data in both ways. Since this is not the case then every replica will use different autogenerated certificates. We have to work on that and try to sync them across replicas somehow.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 26, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floreks, liggitt

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 26, 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/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants