-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Create Keyset API type #3286
Create Keyset API type #3286
Conversation
This will allow management of cluster secrets in kops-server
Splitting up #3150 - should get us cluster rotation, bare metal & kops server support at the end of the road. @chrislovecnm it's a long path, but can you have a look please? |
7edcd21
to
456a863
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not certain if we are not getting code comments because the code was generated, or if you missed some. Not a huge fan or the name, but I cannot think of anything else.
@@ -46,6 +47,10 @@ func (c *KopsClient) InstanceGroups(namespace string) InstanceGroupInterface { | |||
return newInstanceGroups(c, namespace) | |||
} | |||
|
|||
func (c *KopsClient) Keysets(namespace string) KeysetInterface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need code comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generated
|
||
var keysetsKind = schema.GroupVersionKind{Group: "kops", Version: "v1alpha2", Kind: "Keyset"} | ||
|
||
func (c *FakeKeysets) Create(keyset *v1alpha2.Keyset) (result *v1alpha2.Keyset, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this generated? If not we need code comments please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generated
|
||
// +genclient=true | ||
|
||
// Keyset is a set of system keypairs, or other secret material. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to regen the api docs as well, but another PR.
Been super busy. Have not had a chance to look at the other PR yet ... |
So I think you reviewed the generated code - it's only 8792323 that really needs attention. LGTM therefore? |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chrislovecnm, justinsb 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 |
/test all [submit-queue is verifying that this PR is safe to merge] |
/retest |
/test all [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
A Keyset holds a set of keypairs or other secret cluster material.
It is a set to support rotation of keys.
This will allow us to store secrets on kops-server (and also is a step towards
separating where we manage secrets from how we communicate them to running
clusters, which will allow bare-metal or KMS)
Starting with just the API objects.