Use newer x/sys/windows SecurityAttributes struct (carry 40017) by thaJeztah · Pull Request #40021 · moby/moby

thaJeztah · 2019-09-30T19:03:23Z

carries #40017
closes #40017

changes since #40017:

split the vendor bump in a separate commit ()
- full diff of x/sys upstream: full diff: golang/sys@9eafafc...c990c68
fixed the remaining stray import; Use newer x/sys/windows SecurityAttributes struct #40017 (comment)

This struct now has a properly typed member, so use the properly typed functions with it.

zx2c4 · 2019-10-01T08:45:42Z

LGTM. 👍

thaJeztah · 2019-10-01T14:17:23Z

@tklauser @vikramhh @kolyshkin PTAL

tklauser · 2019-10-01T14:37:06Z

LGTM, but I'd wait for the outcome of the discussion on golang/go#34610 where the decision might be to revert the change to SecurityAttributes and introduce a new struct type with a different name.

thaJeztah · 2019-10-01T14:58:41Z

Thanks @tklauser - let me mark this PR as "WIP" in the meantime. I'll also comment on the PR in Microsoft/go-winio

full diff: golang/sys@9eafafc...c990c68 Signed-off-by: Sebastiaan van Stijn <[email protected]>

This struct now has a properly typed member, so use the properly typed functions with it. Also update the vendor directory and hope nothing explodes. Signed-off-by: Jason A. Donenfeld <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>

armandgrillet · 2019-11-05T10:26:11Z

According to golang/go#34610 (comment) and the status of the issue (i.e. stale), the change will not be reverted.

tklauser · 2019-11-08T08:46:36Z

@thaJeztah This is good to go now that golang/go#34610 has been closed and it was decided there will be no revert (golang/go#34610 (comment))

thaJeztah · 2019-11-08T17:30:15Z

@armandgrillet @tklauser thanks for the heads-up; I removed "do not merge" from the title, and triggered CI

armandgrillet · 2019-11-21T09:10:54Z

CI looks green 🎉, @tonistiigi can you please review this PR?

full diff: moby/moby@a09e6e3...a9507c6 Includes: - moby/moby#40077 Update "auto-generate" comments to improve detection by linters - moby/moby#40143 registry: add a critical section to protect authTransport.modReq - moby/moby#40212 Move DefaultCapabilities() to caps package - moby/moby#40021 Use newer x/sys/windows SecurityAttributes struct (carry 40017) - carries moby/moby#40017 Use newer x/sys/windows SecurityAttributes struct - moby/moby#40135 pkg/system: make OSVersion an alias for hcsshim OSVersion - follow-up to moby/moby#39100 Use Microsoft/hcsshim constants and deprecate pkg/system.GetOsVersion() - moby/moby#40250 Bump hcsshim to b3f49c06ffaeef24d09c6c08ec8ec8425a0303e2 - moby/moby#40243 Use certs.d from XDG_CONFIG_HOME when in rootless mode - fixes moby/moby#40236 Docker rootless dies when unable to read /etc/docker/certs.d - moby/moby#40283 Fix possible runtime panic in Lgetxattr - moby/moby#40178 builder/remotecontext: small refactor - moby/moby#40179 builder/remotecontext: allow ssh:// for remote context URLs - fixes docker#2164 Docker build cannot resolve git context with html escapes - moby/moby#40302 client.ImagePush(): default to ":latest" instead of "all tags" - relates to docker#2214 [proposal] change "docker push" behavior to default to ":latest" instead of "all tags" - relates to docker#2220 implement docker push `-a`/ `--all-tags` - moby/moby#40263 Normalize comment formatting - moby/moby#40238 Allow client consumers like traefik to compile on illumos - moby/moby#40108 bump google.golang.org/grpc v1.23.1 - moby/moby#40312 update vendor golang.org/x/sys to 6d18c012aee9febd81bbf9806760c8c4480e870d - moby/moby#40247 pkg/system: deprecate constants in favor of golang.org/x/sys/windows - moby/moby#40246 pkg/system: minor cleanups and remove use of deprecated system.GetOSVersion() - moby/moby#40122 Update buildkit to containerd leases - vendor: update buildkit to leases support (4f4e03067523b2fc5ca2f17514a5e75ad63e02fb) - vendor: update containerd to acdcf13d5eaf0dfe0eaeabe7194a82535549bc2b - vendor: update runc to d736ef14f0288d6993a1845745d6756cfc9ddd5a (v1.0.0-rc9) Signed-off-by: Sebastiaan van Stijn <[email protected]>

full diff: moby/moby@a09e6e3...a9507c6 Includes: - moby/moby#40077 Update "auto-generate" comments to improve detection by linters - moby/moby#40143 registry: add a critical section to protect authTransport.modReq - moby/moby#40212 Move DefaultCapabilities() to caps package - moby/moby#40021 Use newer x/sys/windows SecurityAttributes struct (carry 40017) - carries moby/moby#40017 Use newer x/sys/windows SecurityAttributes struct - moby/moby#40135 pkg/system: make OSVersion an alias for hcsshim OSVersion - follow-up to moby/moby#39100 Use Microsoft/hcsshim constants and deprecate pkg/system.GetOsVersion() - moby/moby#40250 Bump hcsshim to b3f49c06ffaeef24d09c6c08ec8ec8425a0303e2 - moby/moby#40243 Use certs.d from XDG_CONFIG_HOME when in rootless mode - fixes moby/moby#40236 Docker rootless dies when unable to read /etc/docker/certs.d - moby/moby#40283 Fix possible runtime panic in Lgetxattr - moby/moby#40178 builder/remotecontext: small refactor - moby/moby#40179 builder/remotecontext: allow ssh:// for remote context URLs - fixes docker/cli#2164 Docker build cannot resolve git context with html escapes - moby/moby#40302 client.ImagePush(): default to ":latest" instead of "all tags" - relates to docker/cli#2214 [proposal] change "docker push" behavior to default to ":latest" instead of "all tags" - relates to docker/cli#2220 implement docker push `-a`/ `--all-tags` - moby/moby#40263 Normalize comment formatting - moby/moby#40238 Allow client consumers like traefik to compile on illumos - moby/moby#40108 bump google.golang.org/grpc v1.23.1 - moby/moby#40312 update vendor golang.org/x/sys to 6d18c012aee9febd81bbf9806760c8c4480e870d - moby/moby#40247 pkg/system: deprecate constants in favor of golang.org/x/sys/windows - moby/moby#40246 pkg/system: minor cleanups and remove use of deprecated system.GetOSVersion() - moby/moby#40122 Update buildkit to containerd leases - vendor: update buildkit to leases support (4f4e03067523b2fc5ca2f17514a5e75ad63e02fb) - vendor: update containerd to acdcf13d5eaf0dfe0eaeabe7194a82535549bc2b - vendor: update runc to d736ef14f0288d6993a1845745d6756cfc9ddd5a (v1.0.0-rc9) Signed-off-by: Sebastiaan van Stijn <[email protected]> Upstream-commit: 627a4cf7ccd0b7e92c6798c73de4dd4efc43175c Component: cli

Use docker master branch for specific branches, so we can pick up the fix in moby/moby#40021 .

Signed-off-by: Yoan Blanc <[email protected]>

full diff: moby/moby@a09e6e3...a9507c6 Includes: - moby/moby#40077 Update "auto-generate" comments to improve detection by linters - moby/moby#40143 registry: add a critical section to protect authTransport.modReq - moby/moby#40212 Move DefaultCapabilities() to caps package - moby/moby#40021 Use newer x/sys/windows SecurityAttributes struct (carry 40017) - carries moby/moby#40017 Use newer x/sys/windows SecurityAttributes struct - moby/moby#40135 pkg/system: make OSVersion an alias for hcsshim OSVersion - follow-up to moby/moby#39100 Use Microsoft/hcsshim constants and deprecate pkg/system.GetOsVersion() - moby/moby#40250 Bump hcsshim to b3f49c06ffaeef24d09c6c08ec8ec8425a0303e2 - moby/moby#40243 Use certs.d from XDG_CONFIG_HOME when in rootless mode - fixes moby/moby#40236 Docker rootless dies when unable to read /etc/docker/certs.d - moby/moby#40283 Fix possible runtime panic in Lgetxattr - moby/moby#40178 builder/remotecontext: small refactor - moby/moby#40179 builder/remotecontext: allow ssh:// for remote context URLs - fixes docker#2164 Docker build cannot resolve git context with html escapes - moby/moby#40302 client.ImagePush(): default to ":latest" instead of "all tags" - relates to docker#2214 [proposal] change "docker push" behavior to default to ":latest" instead of "all tags" - relates to docker#2220 implement docker push `-a`/ `--all-tags` - moby/moby#40263 Normalize comment formatting - moby/moby#40238 Allow client consumers like traefik to compile on illumos - moby/moby#40108 bump google.golang.org/grpc v1.23.1 - moby/moby#40312 update vendor golang.org/x/sys to 6d18c012aee9febd81bbf9806760c8c4480e870d - moby/moby#40247 pkg/system: deprecate constants in favor of golang.org/x/sys/windows - moby/moby#40246 pkg/system: minor cleanups and remove use of deprecated system.GetOSVersion() - moby/moby#40122 Update buildkit to containerd leases - vendor: update buildkit to leases support (4f4e03067523b2fc5ca2f17514a5e75ad63e02fb) - vendor: update containerd to acdcf13d5eaf0dfe0eaeabe7194a82535549bc2b - vendor: update runc to d736ef14f0288d6993a1845745d6756cfc9ddd5a (v1.0.0-rc9) Signed-off-by: Sebastiaan van Stijn <[email protected]>

This change set addresses #94 and ports baur to Windows. The `make release` command will create a _baur.exe_ file. Things to note: 1. A [breaking change](golang/go#34610) was introduced to golang.org/x/sys that prevents github.com/docker/docker from compiling. This issue in github.com/docker/docker will not be [fixed](moby/moby#40021) until v20 of github.com/docker/docker is released. In the meantime I have pinned golang.org/x/sys to the latest version before the breaking change was introduced. 2. Some of the existing tests used commands specific to Unix. These have either been replaced with equivalent commands available directly on both Unix and Windows or run via `cmd /C` on Windows. 3. Windows does not allow files that are in use to be deleted in the same way as Unix. In some tests files and folders had to be released so the test cleanup succeeds. 4. Expected paths have been updated to use OS specific path separators where required. 5. CircleCI does not provide Windows containers but does provide Windows VMs * The pre-installed version of golang is 1.12.7, this needs to be updated on every build. See [software-pre-installed-in-the-windows-image](https://circleci.com/docs/2.0/hello-world-windows/#software-pre-installed-in-the-windows-image) * I could not access Unix docker containers from within the Windows job so postgres is installed on the VM via chocolatey and the default "postgres" database is used. * The tests are much slower to run on Windows than Unix, the test timeout has been increased to accommodate this. * The `-race` flag has been removed from the Windows `go test` command as a gcc compiler is not installed. I figured this wasn't a problem as it is used in the existing test job. This could be addressed later if necessary.

docker/docker has to be upgraded to have moby/moby#40021. Signed-off-by: Kazuyoshi Kato <[email protected]>

thaJeztah requested a review from tonistiigi as a code owner September 30, 2019 19:03

thaJeztah added platform/windows status/2-code-review labels Sep 30, 2019

thaJeztah mentioned this pull request Sep 30, 2019

Use newer x/sys/windows SecurityAttributes struct #40017

Closed

zx2c4 mentioned this pull request Oct 1, 2019

x/sys/windows: API break in SecurityAttributes struct golang/go#34610

Closed

thaJeztah changed the title ~~Use newer x/sys/windows SecurityAttributes struct (carry 40017)~~ [WIP] Use newer x/sys/windows SecurityAttributes struct (carry 40017) Oct 1, 2019

thaJeztah mentioned this pull request Oct 1, 2019

Move to upstream Sddl/SecurityAttribute functions microsoft/go-winio#165

Open

thaJeztah and others added 2 commits October 2, 2019 21:12

bump golang.org/x/sys c990c680b611ac1aeb7d8f2af94a825f98d69720

4e0836f

full diff: golang/sys@9eafafc...c990c68 Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah force-pushed the carry_40017 branch from b7b9e08 to c3a0a37 Compare October 2, 2019 19:12

thaJeztah changed the title ~~[WIP] Use newer x/sys/windows SecurityAttributes struct (carry 40017)~~ [do not merge] Use newer x/sys/windows SecurityAttributes struct (carry 40017) Oct 3, 2019

thaJeztah added the kind/refactor PR's that refactor, or clean-up code label Oct 10, 2019

thaJeztah changed the title ~~[do not merge] Use newer x/sys/windows SecurityAttributes struct (carry 40017)~~ Use newer x/sys/windows SecurityAttributes struct (carry 40017) Nov 8, 2019

tiborvass approved these changes Nov 21, 2019

View reviewed changes

tonistiigi approved these changes Nov 21, 2019

View reviewed changes

tonistiigi merged commit d1d5f64 into moby:master Nov 21, 2019

thaJeztah deleted the carry_40017 branch November 21, 2019 19:05

nonsense mentioned this pull request Nov 27, 2019

initial daemon implementation testground/testground#183

Merged

9 tasks

thaJeztah mentioned this pull request Jan 7, 2020

vendor: bump docker and dependencies a9507c6f76627fdc092edc542d5a7ef4a6df5eec docker/cli#2242

Merged

greut mentioned this pull request Mar 15, 2020

Docker 19.03.8 hashicorp/nomad#7351

Closed

1 task

notnoop pushed a commit to hashicorp/nomad that referenced this pull request Mar 30, 2020

Fix docker/docker compile issue

a5e16de

Use docker master branch for specific branches, so we can pick up the fix in moby/moby#40021 .

thaJeztah added this to the 20.03.0 milestone Apr 2, 2020

greut added a commit to greut/nomad that referenced this pull request Jun 1, 2020

docker pick moby/moby#40021

1b61b16

Signed-off-by: Yoan Blanc <[email protected]>

kasey mentioned this pull request Sep 28, 2020

crank build, lint and push crossplane/crossplane#1760

Merged

2 tasks

This was referenced Oct 12, 2020

Compile and run on Windows simplesurance/baur#186

Closed

Port to Windows simplesurance/baur#215

Closed

dil-spayne mentioned this pull request Oct 20, 2020

Port to Windows simplesurance/baur#216

Merged

This was referenced Jan 7, 2021

feat: migrate to tf-sdk2 kreuzwerker/terraform-provider-docker#102

Merged

chore: bump docker dependency to v20.10.5 kreuzwerker/terraform-provider-docker#119

Merged

kzys pushed a commit to kzys/amazon-ecs-agent that referenced this pull request Nov 2, 2022

Upgrade github.com/docker/docker to v20.10.21

04fd7f4

docker/docker has to be upgraded to have moby/moby#40021. Signed-off-by: Kazuyoshi Kato <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use newer x/sys/windows SecurityAttributes struct (carry 40017)#40021

Use newer x/sys/windows SecurityAttributes struct (carry 40017)#40021
tonistiigi merged 2 commits intomoby:masterfrom
thaJeztah:carry_40017

thaJeztah commented Sep 30, 2019

Uh oh!

zx2c4 commented Oct 1, 2019

Uh oh!

thaJeztah commented Oct 1, 2019

Uh oh!

tklauser commented Oct 1, 2019

Uh oh!

thaJeztah commented Oct 1, 2019

Uh oh!

armandgrillet commented Nov 5, 2019

Uh oh!

tklauser commented Nov 8, 2019

Uh oh!

thaJeztah commented Nov 8, 2019

Uh oh!

armandgrillet commented Nov 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

thaJeztah commented Sep 30, 2019

Uh oh!

zx2c4 commented Oct 1, 2019

Uh oh!

thaJeztah commented Oct 1, 2019

Uh oh!

tklauser commented Oct 1, 2019

Uh oh!

thaJeztah commented Oct 1, 2019

Uh oh!

armandgrillet commented Nov 5, 2019

Uh oh!

tklauser commented Nov 8, 2019

Uh oh!

thaJeztah commented Nov 8, 2019

Uh oh!

armandgrillet commented Nov 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants