Address code review comments, and document the secret protos better

Signed-off-by: cyli <cyli@twistedmatrix.com>

api/objects.proto

@@ -223,33 +223,53 @@ message Cluster {
 	uint64 encryption_key_lamport_clock = 6;
 }
 
-// Secret
+// Secret represents a conceptual secret that should be passed to a container.
+// It can contain many versions of secret data, since conceptually secret
+// credentials can be rotated.
 message Secret {
 	string id = 1 [(gogoproto.customname) = "ID"];
 
 	Meta meta = 2 [(gogoproto.nullable) = false];
 
+	// A secret has multiple versions, and each version is represented by a
+	// `SecretData` object and referenced by the `SecretData`'s ID.  This map
+	// stores `SecretData` objects keyed by ID, which lets us reference any
+	// particular version of a secret easily.
 	map<string, SecretData> secret_data = 3;
 
-	// Name of the secret, so that we can use it easily for lookups
+	// Name of the secret, so that we can use it easily for lookups.  This is
+	// just a copy of the data in `SecretData.Spec.Annotations.Name`.
 	string name = 4;
 
-	// ID of the latest version, so that we can easily use it for lookups
+	// Latest is the cached ID of the latest version, so that we can easily use
+	// it for lookups without having to sort the `SecretData` map, although
+	// we will have to sort and get the latest version if the `LatestVersion`
+	// is removed.
 	string latest = 5 [(gogoproto.customname) = "LatestVersion"];
 }
 
-// SecretData is represents a specific version of a secret, and is immutable
+// SecretData is represents a specific version of a secret, and is immutable;
+// it wraps the `spec` provided by the user with useful information that is
+// generated from the secret data in the `spec`, such as the digest and size
+// of the secret data.
 message SecretData {
 	string id = 1 [(gogoproto.customname) = "ID"];
 
-	Meta meta = 2 [(gogoproto.nullable) = false];
+	Timestamp created_at = 2 [(gogoproto.nullable) = false];
 
-	SecretSpec spec = 3  [(gogoproto.nullable) = false];
+	// Spec contains the actual secret data, as well as any context around the
+	// secret data that the user provides.  The name on the annotation of the
+	// spec must match the name of the `Secret` object this `SecretData` belongs
+	// to.
+	SecretDataSpec spec = 3  [(gogoproto.nullable) = false];
 
-	// Digest represents the cryptographic digest of the secret data
+	// Digest represents the cryptographic digest of the secret data, and follows
+	// the form "<algorithm>:<digest>": for example "sha256:DEADBEEF...".  It
+	// is calculated from the data contained in `SecretData.Spec.data`.
 	string digest = 4;
 
-	// Size represents the size of the secret
-	int32 size = 5 [(gogoproto.customname) = "SecretSize"];
+	// Size represents the size (number of bytes) of the secret data, and is
+	// calculated from the data contained in `SecretData.Spec.data`..
+	uint32 size = 5 [(gogoproto.customname) = "SecretSize"];
 }
 

api/secrets.proto

@@ -6,35 +6,74 @@ import "specs.proto";
 import "objects.proto";
 import "plugin/plugin.proto";
 
-// Secrets defines the RPC methods for controlling secrets in a cluster.
+// Secrets defines the RPC methods for managing/controlling secrets in a cluster.
 service Secrets {
+	// GetSecret returns a `GetSecretResponse` with a `Secret` with the same
+	// name as `GetSecretRequest.Name`
+	// - Returns `NotFound` if the Secret with the given name is not found.
+	// - Returns `InvalidArgument` if the `GetSecretRequest.Name` is empty.
+	// - Returns an error if getting fails.
 	rpc GetSecret(GetSecretRequest) returns (GetSecretResponse) {
 		option (docker.protobuf.plugin.tls_authorization) = { roles: "swarm-manager" };
 	}
+
+	// ListSecrets returns a `ListSecretResponse` with a list all `Secret`s being
+	// managed, or all secrets matching any name in `ListSecretsRequest.Name` or any
+	// name prefix in `ListSecretsRequest.NamePrefix`
+	// - Returns an error if listing fails.
 	rpc ListSecrets(ListSecretsRequest) returns (ListSecretsResponse) {
 		option (docker.protobuf.plugin.tls_authorization) = { roles: "swarm-manager" };
 	}
+	// CreateSecret creates and return a `CreateSecretResponse` with a `Secret`` based
+	// on the provided `CreateSecretRequest.SecretDataSpec`.
+	// - Returns `InvalidArgument` if the `CreateSecretRequest.SecretDataSpec` is malformed.
+	// - Returns `AlreadyExists` if the `CreateSecretRequest.SecretDataSpec.Annotation.Name`
+	//   conflicts with an existing secret's name.
+	// - Returns an error if the creation fails.
 	rpc CreateSecret(CreateSecretRequest) returns (CreateSecretResponse) {
 		option (docker.protobuf.plugin.tls_authorization) = { roles: "swarm-manager" };
 	}
+
+	// UpdateSecret adds the `UpdateSecretRequest.SecretDataSpec` to a `Secret`` as a new version,
+	// (and is set as the the `Secret.LatestVersion`) and returns the updated
+	// `Secret` in the `UpdateSecretResponse`.
+	// - Returns `NotFound` if no secret with name
+	//   `UpdateSecretRequest.SecretDataSpec.Annotations.Name` is found.
+	// - Returns `InvalidArgument` if the `UpdateSecretRequest.SecretDataSpec` is malformed.
+	// - Returns an error if the update fails.
 	rpc UpdateSecret(UpdateSecretRequest) returns (UpdateSecretResponse) {
 		option (docker.protobuf.plugin.tls_authorization) = { roles: "swarm-manager" };
 	}
+
+	// RemoveSecret removes a referenced by `RemoveSecretRequest.Name`.  If
+	// `RemoveSecretRequest.Version` is not provided, the entire secret and all its
+	// versions are removed.  Otherwise, only the version of the secret referenced
+	// by `RemoveSecretRequest.Version` is removed from the secret.  The
+	// `Secret.LatestVersion` is updated as needed.
+	// - Returns `InvalidArgument` if `RemoveSecretRequest.Name` is empty.
+	// - Returns `NotFound` if the a secret named `RemoveSecretRequest.Name` is not
+	//   found or if it is found but has no version `RemoveSecretRequest.Version`.
+	// - Returns an error if the deletion fails.
 	rpc RemoveSecret(RemoveSecretRequest) returns (RemoveSecretResponse) {
 		option (docker.protobuf.plugin.tls_authorization) = { roles: "swarm-manager" };
 	}
 }
 
+// GetSecretRequest is the request to get a `Secret` object given a secret name.
 message GetSecretRequest {
-	// Name is name of the secret to get info for.
 	string name = 1;
 }
 
+// GetSecretResponse contains the Secret corresponding to the name in
+// `GetSecretRequest`, but the `SecretData.Spec.Data` field in each `SecretData`
+// object in the `Secret` should be nil instead of actually containing
+// the secret bytes.
 message GetSecretResponse {
-	// Secret is the secret corresponding to that ID, but each version in `secret.spec.data` should have its `.data` field be nil
 	Secret secret = 1;
 }
 
+// ListSecretRequest is the request to list all secrets in the secret store,
+// or all secrets filtered by name or name prefix.
 message ListSecretsRequest {
 	message Filters {
 		repeated string names = 1;
@@ -44,34 +83,54 @@ message ListSecretsRequest {
 	Filters filters = 1;
 }
 
+// ListSecretResponse contains a list of all the secrets that match the name or
+// name prefix filters provided in `ListSecretRequest`.  The `SecretData.Spec.Data`
+// field in each `SecretData` object in each `Secret` should be nil instead of
+// actually containing the secret bytes.
 message ListSecretsResponse {
-	// Each Secret is the secret corresponding to that ID, but each version in `secret.spec.data` should have its `.data` field be nil
 	repeated Secret secrets = 1;
 }
 
-// Note CreateSecretRequest does not contain a SecretSpec, since that can contain multiple SecretData objects, which have digests
-// which should be generated by the
+// CreateSecretRequest specifies a new secret (it will not update an existing
+// secret) to create.
 message CreateSecretRequest {
-	SecretSpec spec = 1;
+	SecretDataSpec spec = 1;
 }
 
+// CreateSecretResponse contains the newly created `Secret`` corresponding to the
+// name in `CreateSecretRequest`.  Since it's a new secret, there should just be
+// the one version, and hence only one `SecretData` object in the `Secret`.
+// The `SecretData.Spec.Data` field should be nil instead of actually
+// containing the secret bytes.
 message CreateSecretResponse {
 	Secret secret = 1;
 }
 
+// UpdateSecretRequest specifies a new secret version to add to an existing secret
+// of the same name.
 message UpdateSecretRequest {
-	SecretSpec spec = 1;
+	SecretDataSpec spec = 1;
 }
 
+// UpdateSecretResponse contains the Secret corresponding to the name of the spec
+// provided in `UpdateSecretRequest`, and it should be updated with an additional
+// `SecretData` object containing said spec.  The `SecretData.Spec.Data` field in
+// each `SecretData` object in the `Secret` object should be nil instead of
+// actually containing the secret bytes.
 message UpdateSecretResponse {
 	Secret secret = 1;
 }
 
+// RemoveSecretRequest contains the name and version (optional) of the secret
+// that should be removed.  If no version is provided, the intention is that
+// all versions should be removed.
 message RemoveSecretRequest {
 	// Name is name of the secret to remove.
 	string name = 1;
 	// Version is the version to remove
 	string version = 2;
 }
 
+// RemoveSecretResponse is an empty object indicating the successful removal of
+// a secret.
 message RemoveSecretResponse {}