Add API documentation for plugable secret backends #34202

thaJeztah · 2017-07-20T14:51:57Z

Documents the API changes introduced in

0304c98 (#34157) and 08f7cf0 (#34123)

- What I did

- How I did it

- How to verify it

run make swagger-docs and visit http://localhost:9000 to view the docs

ping @liron-l @cpuguy83 @vdemeester PTAL

tophj-ibm · 2017-07-20T16:09:15Z

api/swagger.yaml

+  Driver:
+    description: "Driver represents a driver (network, logging, secrets)."
+    type: "object"
+    required: [Name, Data]


janky failure looks related.

09:58:42 - The swagger spec at "api/swagger.yaml" is invalid against swagger specification 2.0. see errors : 09:58:42 - "Data" is present in required but not defined as property in definition "Driver" 09:58:42 - /secrets/{id}.Spec.Driver.Data in body is required

Q: I haven't looked at the swagger.yaml before, but I'm guessing Data is supposed to Options here, which is based off

moby/api/types/swarm/common.go

Line 23 in 0304c98

// Driver represents a driver (network, logging).

right?

oh, hm, looks like I may have copy/pasted the Data yes thanks, I'll check

Don't think Data / Options is required here, so I'll remove

thaJeztah · 2017-07-25T08:22:07Z

ping @liron-l @cpuguy83 @vdemeester PTAL

liron-l · 2017-07-25T08:34:29Z

api/swagger.yaml

+        type: "string"
+        x-nullable: false
+        example: "some-driver"
+      Options:


Consider
description: "key/value map of driver specific options." (same as DriverConfig.Options)

liron-l · 2017-07-25T08:35:44Z

docs/api/version-history.md

@@ -19,6 +19,14 @@ keywords: "API, Docker, rcli, REST, documentation"

 * `DELETE /secrets/(name)` now returns status code 404 instead of 500 when the secret does not exist.
 * `POST /secrets/create` now returns status code 409 instead of 500 when creating an already existing secret.
+* `POST /secrets/create` now accepts a `Driver` struct, allowing the
+  `Name` and diver-specific `Options` to be passed to store a secrets


diver-specific -> driver-specific (here and below)

Good catch :)

liron-l · 2017-07-25T08:36:56Z

LGTM @thaJeztah, left some minor comments inline.

thaJeztah · 2017-07-25T09:12:36Z

@liron-l thanks, I updated.

Question: do you know;

What are the name restrictions on Secrets drivers? (e.g., only a-zA-Z0-9_.-)
Is there a maximum length?
Are Secrets driver names case-sensitive?
If none of the above; can we add such validation? (We've been bitten by people relying on no restrictions being there, and once we wanted to add validation, it was a breaking change)

TL;DR I'd like to document what are acceptable values for the Name field :)

liron-l · 2017-07-25T10:12:10Z

Thanks @thaJeztah.
Technically, the restrictions should be on the secrets plugin name, since the driver name is only a reference to the name of the plugin. Also, we should enforce that the specified secret's plugin exist as part of the docker secret create flow (and then we don't need additional name validation).

Finally, if we do add such restrictions on the name, I know that plguins names need to have versioning support (:).
@cpuguy83 does this makes sense?

thaJeztah · 2017-07-25T10:17:16Z

Ah, right, yes, these are managed plugins (installed through docker plugin install)? (sorry, I may be slow, ha!) in that case, we should probably be covered here (can add some information in future)

liron-l · 2017-07-25T13:34:10Z

Yes, @thaJeztah, secret plugins are managed plugins. We will add dedicated documentation and examples after all relevant commits are merged.
docker/cli#366
moby/swarmkit#2326

dnephin · 2017-07-27T20:43:06Z

api/swagger.yaml

+        additionalProperties:
+          type: "string"
+          example:
+            "value for driver-specific option"


This formatting seems wrong. I think there's a key missing here on the newline?

oh, this is odd, yes, definitely should be fixed 👍

dnephin · 2017-07-27T20:44:42Z

api/swagger.yaml

+          data to store as secret.
+
+          This field is only used to _create_ a secret, and is not returned by
+          other endpoints.


Hmm, we should probably have two different types then. One without this field. Since this already exited we can wait for another PR

Yes; while working on these changes in the Swagger, I found other types that are reused and don't make sense

I think this is an "omitempty" (so in practice it's not there), but yes, probably should have a separate type

thaJeztah · 2017-07-27T21:59:37Z

api/swagger.yaml

      Data:
-        description: "Base64-url-safe-encoded secret data"
-        type: "array"


FWIW I changed this from an array to just string; I guess this was because in go it's a []byte

Documents the API changes introduced in 0304c98 and 08f7cf0 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2017-07-27T22:01:23Z

@dnephin updated PTAL

dnephin

LGTM

vdemeester

LGTM 🐸

thaJeztah added area/api area/docs status/3-docs-review labels Jul 20, 2017

GordonTheTurtle added the status/0-triage label Jul 20, 2017

tophj-ibm reviewed Jul 20, 2017

View reviewed changes

thaJeztah force-pushed the swagger-external-secrets-store branch from cb04757 to 22c654e Compare July 20, 2017 16:25

thaJeztah removed the status/0-triage label Jul 25, 2017

liron-l reviewed Jul 25, 2017

View reviewed changes

thaJeztah force-pushed the swagger-external-secrets-store branch from 22c654e to 9f19ba7 Compare July 25, 2017 09:09

dnephin reviewed Jul 27, 2017

View reviewed changes

thaJeztah force-pushed the swagger-external-secrets-store branch 2 times, most recently from 422c690 to 417a9a7 Compare July 27, 2017 21:58

thaJeztah commented Jul 27, 2017

View reviewed changes

Add API documentation for plugable secret backends

c8dad44

Documents the API changes introduced in 0304c98 and 08f7cf0 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the swagger-external-secrets-store branch from 417a9a7 to c8dad44 Compare July 27, 2017 22:01

dnephin approved these changes Jul 28, 2017

View reviewed changes

vdemeester approved these changes Aug 1, 2017

View reviewed changes

vdemeester merged commit 0fd90c4 into moby:master Aug 1, 2017

thaJeztah deleted the swagger-external-secrets-store branch August 1, 2017 11:41

thaJeztah mentioned this pull request Aug 1, 2017

[17.07] backport plugable secret api docs docker-archive/docker-ce#155

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add API documentation for plugable secret backends #34202

Add API documentation for plugable secret backends #34202

thaJeztah commented Jul 20, 2017

tophj-ibm Jul 20, 2017

thaJeztah Jul 20, 2017

thaJeztah Jul 20, 2017

thaJeztah commented Jul 25, 2017

liron-l Jul 25, 2017

liron-l Jul 25, 2017

thaJeztah Jul 25, 2017

liron-l commented Jul 25, 2017

thaJeztah commented Jul 25, 2017

liron-l commented Jul 25, 2017

thaJeztah commented Jul 25, 2017

liron-l commented Jul 25, 2017

dnephin Jul 27, 2017

thaJeztah Jul 27, 2017

dnephin Jul 27, 2017

thaJeztah Jul 27, 2017

thaJeztah Jul 27, 2017

thaJeztah commented Jul 27, 2017

dnephin left a comment

vdemeester left a comment

Add API documentation for plugable secret backends #34202

Add API documentation for plugable secret backends #34202

Conversation

thaJeztah commented Jul 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah commented Jul 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liron-l commented Jul 25, 2017

thaJeztah commented Jul 25, 2017

liron-l commented Jul 25, 2017

thaJeztah commented Jul 25, 2017

liron-l commented Jul 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah commented Jul 27, 2017

dnephin left a comment

Choose a reason for hiding this comment

vdemeester left a comment

Choose a reason for hiding this comment