Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Allow option to disable certificates management #243

Merged

Conversation

redbaron
Copy link
Contributor

No description provided.

@redbaron redbaron force-pushed the disable-cert-management branch from d8db46b to 217e95c Compare January 12, 2017 18:59
@codecov-io
Copy link

codecov-io commented Jan 12, 2017

Current coverage is 68.54% (diff: 75.00%)

Merging #243 into master will increase coverage by 0.08%

@@             master       #243   diff @@
==========================================
  Files             4          4          
  Lines          1100       1103     +3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            753        756     +3   
  Misses          261        261          
  Partials         86         86          

Powered by Codecov. Last update bc1e6ed...1239526

@@ -46,6 +46,11 @@ availabilityZone: {{.AvailabilityZone}}
# ARN of the KMS key used to encrypt TLS assets.
kmsKeyArn: "{{.KMSKeyARN}}"

# If you do not want kube-aws to manage certificaes, set it to false. If you do that
# you are responsible for making sure that nodes have correct certificates by the time
# daemons start up.
Copy link
Contributor

Choose a reason for hiding this comment

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

An example (or a link to a doc explaining it) of how to feed certificates to nodes before daemons start up would be helpful. I'm not really sure how myself and so are others!

Also, a brief explanation of when this feature is intended to be used (a.k.a why "you do not want kube-aws to manage certificates" in the first place) would be good to add, too.

return nil, err
}
stackConfig.Config.TLSConfig = compactAssets
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a test for this?
I believe TestReadOrCreateCompactTLSAssets would be an useful reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you like to me add the tests instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please adopt / modify any of my PRs if it can speedup merging them to master

@mumoshu
Copy link
Contributor

mumoshu commented Jan 13, 2017

@redbaron Thanks as always! I'm impressed to the amount of contribution you're doing these days 👍
LGTM overall but two comments regarding the original intention of adding this feature and testing.

@mumoshu mumoshu merged commit 0276ee2 into kubernetes-retired:master Jan 18, 2017
@mumoshu mumoshu added this to the v0.9.4-rc.1 milestone Jan 18, 2017
camilb added a commit to camilb/kube-aws that referenced this pull request Feb 28, 2017
* coreos/master: (132 commits)
  fix: Spot Fleet doesn't support the t2 instance family
  Fix node pools on master
  Allow option to disable certificates management (kubernetes-retired#243)
  Bump to k8s 1.5.2
  Update README.md
  Update ROADMAP.md
  Update ROADMAP.md
  Update ROADMAP.md
  Update the inline documentation in cluster.yaml
  typo
  Don't fail sed if some files are missing
  Workaround systemd issues with oneshot autorestarts
  etcd static IP addressing overhaul
  Calico self hosted integration (kubernetes-retired#124)
  Fix lint.
  bugfix for a typo in install-kube-system scripts
  Update README.md
  fix(e2e): Correctly wait for a node pool stack for deletion
  Don't require key-name param during cluster init
  Propagate SSHAuthorizedKeys to nodepools
  ...
camilb added a commit to camilb/kube-aws that referenced this pull request Apr 21, 2017
* coreos/master: (49 commits)
  fix: Spot Fleet doesn't support the t2 instance family
  Fix node pools on master
  Allow option to disable certificates management (kubernetes-retired#243)
  Bump to k8s 1.5.2
  Update README.md
  Update ROADMAP.md
  Update ROADMAP.md
  Update ROADMAP.md
  Update the inline documentation in cluster.yaml
  typo
  Don't fail sed if some files are missing
  Workaround systemd issues with oneshot autorestarts
  etcd static IP addressing overhaul
  Calico self hosted integration (kubernetes-retired#124)
  Fix lint.
  bugfix for a typo in install-kube-system scripts
  Update README.md
  fix(e2e): Correctly wait for a node pool stack for deletion
  Don't require key-name param during cluster init
  Propagate SSHAuthorizedKeys to nodepools
  ...
kylehodgetts pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Mar 27, 2018
* Allow option to disable certificates management
* Don't create KMS policies if we don't use them
@kylehodgetts kylehodgetts deleted the disable-cert-management branch April 8, 2018 10:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants