Windows: Add CredentialSpec #814

lowenna · 2017-05-12T21:40:57Z

Signed-off-by: John Howard jhoward@microsoft.com

The Windows implementation of the docker daemon still passes a number of fields out of band from the OCI runtime spec. This PR addresses the CredentialSpec field by adding it to the runtime spec. (https://github.com/moby/moby/blob/master/libcontainerd/client_windows.go#L177-L178)

wking · 2017-05-12T23:19:31Z

config-windows.md

+
+## <a name="configWindowsCredentialSpec" />Credential Spec
+
+You can configure a container's group Managed Service Accounts (gMSAs) via the OPTIONAL `credentialspec` field of the Windows configuration. For more information about gMSAs, see [Active Directory Service Accounts for Windows Containers][gMSAOverview]. For more information about tooling to generate a gMSA, see [Deployment Overview][gMSATooling]. 


Travis doesn't like the trailing whitespace here (or after "GroupManagedServiceAccounts": [ below).

Ah, yes, fixing.

wking · 2017-05-12T23:21:50Z

config-windows.md

+
+```json
+    "windows": {
+        "credentialspec": {


This value is a JSON object. Your current docs don't have much to say on the value type (using the “credentialspec (object, OPTIONAL) …” pattern would help with this), but your current Go type has it listed as a string (which will break if you try to read in your example here).

It's an opaque JSON blob passed as a string. I should have escaped the example and will shortly. The actual contents are very independent from the spec and implementation specific which is why I referred to the tooling to create such an example. I really don't believe the contents should be described in this spec.

lowenna · 2017-05-13T03:47:20Z

Added matching change to schema\config-windows.json

wking · 2017-05-13T05:29:38Z

config-windows.md

+
+## <a name="configWindowsCredentialSpec" />Credential Spec
+
+You can configure a container's group Managed Service Accounts (gMSAs) via the OPTIONAL `credentialspec` field of the Windows configuration. For more information about gMSAs, see [Active Directory Service Accounts for Windows Containers][gMSAOverview]. For more information about tooling to generate a gMSA, see [Deployment Overview][gMSATooling]. The `credentialspec` MUST be a string containing an escaped JSON object.


Instead of making this a string with serialized JSON, can you make it a JSON object, say the object properties are implementation-defined, and use an interface{} in the Go type? That should get you the same flexibility while still letting you (de)serialize in a single pass.

Yes, I can change this to be interface{}. (Done). I'm not sure what this should be in schema\config-windows.json though if not type string? This is unique - no similar example exists in the spec to duplicate.

I have also updated the description in config-windows.md to indicate it's implementation-defined.

To be clear though - you are recommending to remove the example (even though technically accurate)? And what about the links? Should they stay (I think they should...)?

I'm not sure what this should be in schema\config-windows.json though if not type string?

You can just declare it an object and leave it at that (backing RFC drafts here and here):

"credentialspec": { "id": "https://opencontainers.org/schema/bundle/windows/credentialspec", "type": "object" }

To be clear though - you are recommending to remove the example (even though technically accurate)?

Ideally we'd link to the backing Windows docs (which are necessary to satisfy “implementation-defined”. Lacking that, I'm in favor of including as much information as we can as long as we're reasonably confident that it won't go stale during the useful lifetime of the next spec release. If you expect the links and example you have now to be current for the lifetime of the 1.0 runtime spec, I'm fine with them. If you expect they will go stale before runtime-spec 1.0 is archaic, I'd rather replace them with something generic enough to stay current.

You can just declare it an object and leave it at that

Thanks. Done.

While I don't believe there's any reason that the example will be invalid (as it's already in a released Microsoft product that customers are using), on reflection, it probably is cleaner to remove the example and refer readers back to the source of knowledge, that being the Microsoft documentation. Update done and pushed.

For the links, @PatrickLang is going to arrange a couple of aka.ms redirects, and I'll update the PR with them once I have them.

Signed-off-by: John Howard <jhoward@microsoft.com>

lowenna · 2017-05-15T23:29:00Z

Updated to aka.ms links. Thanks @PatrickLang 😸

TomSweeneyRedHat · 2017-05-16T10:52:14Z

LGTM

mrunalp · 2017-05-18T18:14:43Z

LGTM

tianon · 2017-05-18T18:15:38Z

LGTM

Through f4d221c (Merge pull request opencontainers#880 from dqminh/wking-linux-only-capabilities-again, 2017-07-05). The rc6 release picked up an earlier version of these notes, and those entries are mostly unchanged except for: * The credentialSpec entry, which was opencontainers#814 for credentialspec and now also includes opencontainers#859 for credentialSpec. * The root(.path) Hyper-V entry, which was opencontainers#820 for root.path and now also includes opencontainers#838 for root. I also moved this into the "breaking changes" section, because rc5 Hyper-V configs required root to be set, and rc6 Hyper-V configs require it to not be set. Although whether rc5 allowed Hyper-V configs at all is not clear to me. * Fixed indenting for the typo-fixes entry, as well as a number of more recent typo-fix PRs. Signed-off-by: W. Trevor King <wking@tremily.us>

This was referenced May 12, 2017

Windows: Add servicing #815

Merged

Windows: Add IgnoreFlushesDuringBoot #816

Merged

wking mentioned this pull request May 12, 2017

Windows targets for 1.0? #817

Closed

lowenna mentioned this pull request May 12, 2017

Windows: Add Hyper-V isolation fields #818

Merged

wking reviewed May 12, 2017

View reviewed changes

lowenna force-pushed the credentialspec branch 2 times, most recently from a7b7359 to 93a4dce Compare May 13, 2017 03:47

lowenna force-pushed the credentialspec branch from 93a4dce to 066bc23 Compare May 13, 2017 03:48

wking reviewed May 13, 2017

View reviewed changes

lowenna force-pushed the credentialspec branch 4 times, most recently from c7365cb to 1aad32c Compare May 15, 2017 22:29

lowenna mentioned this pull request May 15, 2017

Add LayerFolders to Windows platform config #828

Merged

lowenna force-pushed the credentialspec branch from 1aad32c to 7b3d15a Compare May 15, 2017 23:19

Windows: Add CredentialSpec

5a9490a

Signed-off-by: John Howard <jhoward@microsoft.com>

lowenna force-pushed the credentialspec branch from 7b3d15a to 5a9490a Compare May 15, 2017 23:28

vbatts added the platforms/windows label May 17, 2017

mrunalp approved these changes May 18, 2017

View reviewed changes

tianon merged commit 5753194 into opencontainers:master May 18, 2017

vbatts mentioned this pull request May 19, 2017

Bump version (rc6 and back to dev) #842

Closed

wking mentioned this pull request Jun 1, 2017

Add readonly option for mount #868

Merged

vbatts mentioned this pull request Jul 5, 2017

bump to v1.0.0-rc6 #882

Merged

wking mentioned this pull request Jul 10, 2017

ChangeLog: Document changes since c83b8c8 #888

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows: Add CredentialSpec #814

Windows: Add CredentialSpec #814

lowenna commented May 12, 2017

wking May 12, 2017

lowenna May 13, 2017

wking May 12, 2017 •

edited

Loading

lowenna May 13, 2017

lowenna commented May 13, 2017

wking May 13, 2017

lowenna May 15, 2017

wking May 15, 2017

lowenna May 15, 2017

lowenna commented May 15, 2017

TomSweeneyRedHat commented May 16, 2017

mrunalp commented May 18, 2017 •

edited by caniszczyk

Loading

tianon commented May 18, 2017 •

edited by caniszczyk

Loading


		## <a name="configWindowsCredentialSpec" />Credential Spec

		You can configure a container's group Managed Service Accounts (gMSAs) via the OPTIONAL `credentialspec` field of the Windows configuration. For more information about gMSAs, see [Active Directory Service Accounts for Windows Containers][gMSAOverview]. For more information about tooling to generate a gMSA, see [Deployment Overview][gMSATooling].

Windows: Add CredentialSpec #814

Windows: Add CredentialSpec #814

Conversation

lowenna commented May 12, 2017

wking May 12, 2017

Choose a reason for hiding this comment

lowenna May 13, 2017

Choose a reason for hiding this comment

wking May 12, 2017 • edited Loading

Choose a reason for hiding this comment

lowenna May 13, 2017

Choose a reason for hiding this comment

lowenna commented May 13, 2017

wking May 13, 2017

Choose a reason for hiding this comment

lowenna May 15, 2017

Choose a reason for hiding this comment

wking May 15, 2017

Choose a reason for hiding this comment

lowenna May 15, 2017

Choose a reason for hiding this comment

lowenna commented May 15, 2017

TomSweeneyRedHat commented May 16, 2017

mrunalp commented May 18, 2017 • edited by caniszczyk Loading

tianon commented May 18, 2017 • edited by caniszczyk Loading

wking May 12, 2017 •

edited

Loading

mrunalp commented May 18, 2017 •

edited by caniszczyk

Loading

tianon commented May 18, 2017 •

edited by caniszczyk

Loading