Rename to citadel #23
Conversation
This change only moves files and updates import paths. It does change the behavior of any APIs. The code continues to compile. Signed-off-by: Monis Khan <[email protected]>
| runtimeName = "kms_cmd" | ||
| runtimeVersion = "0.0.1" // TODO embed git sha on build | ||
| ) | ||
| const kmsAPIVersion = "v1beta1" | ||
|
|
||
| var apiVersionResponse = &v1beta1.VersionResponse{ |
There was a problem hiding this comment.
I assume this is what you are renaming later?
There was a problem hiding this comment.
as its still kms in this file.
There was a problem hiding this comment.
I actually plan to make a v1beta1 folder and, in the future, a v1 folder (so a single daemon process will support all versions of the KMS API).
There was a problem hiding this comment.
This kms is correct because it refers to the Kube API only.
There was a problem hiding this comment.
+1 for the rename, then.
😄
pkg/cmd/citadel/options.go
Outdated
| @@ -48,7 +48,7 @@ var ( | |||
| ) | |||
|
|
|||
| func init() { | |||
| flag.StringVar(&args.endpoint, "endpoint", "", `the listen address (ex. unix:///tmp/kms.sock)`) | |||
| flag.StringVar(&args.endpoint, "endpoint", "", `the listen address (ex. unix:///tmp/socketfile.sock)`) | |||
There was a problem hiding this comment.
This undoes my previous change. I'm trying to keep doc strings short (for an 80 character terminal). Also, "socketfile.sock" is redundant.
There was a problem hiding this comment.
Trying to remove the use of "kms", and this was what the kube examples used. I can pick something else that is short.
|
|
||
| KMS is a simple daemon that implements the Kubernetes Key Management Service | ||
| Citadel (c5l) is a simple daemon that implements the Kubernetes Key Management Service (KMS) |
There was a problem hiding this comment.
I don't like the c5l abbreviation. citadel isn't that long.
There was a problem hiding this comment.
I used it as the short name in the prefix code (to mimic kube), so I wanted to actually use it so people would know what it was referring to.
I am not really worried about the length in the docs, it is more of "google will find it"
There was a problem hiding this comment.
I still don't like it, but if you want to mimic kube, I can accept that.
README.md
Outdated
| @@ -29,7 +29,7 @@ socket activation is assumed. | |||
|
|
|||
| ### Optional | |||
|
|
|||
| * `--endpoint string`: the listen address (ex. `unix:///tmp/kms.sock`) | |||
| * `--endpoint string`: the listen address (ex. `unix:///tmp/socketfile.sock`) | |||
There was a problem hiding this comment.
See my previous comment about length and redundancy.
This change breaks the existing prefix API by changing the short name used to "c5l" instead of "ck" - hopefully for the last time. The string "kms" is now exclusively used to refer to the Kubernetes interface and the daemon correctly identifies itself as "citadel." Signed-off-by: Monis Khan <[email protected]>
This change updates the README to refer to the daemon as "citadel" or "c5l" for short. KMS now only refers to the Kubernetes interface. Signed-off-by: Monis Khan <[email protected]>
|
@npmccallum PTAL |
Fixes #22
@npmccallum I will rename the repo after this merges.
cc @lancejjohnson