Add escape hatch for GODEBUG=x509negativeserial #6371

Benehiko · 2025-08-26T13:23:47Z

- What I did
Negative serial number certificates are deprecated and Go has also since deprecated them. To use them with Go you need to pass in the GODEBUG=x509negativeserial=1 environment variable.

This PR allows setting the value inside docker context instead. This should then propagate environment variables to underlying plugins (such as buildx).

{
  "Name": "my-context",
  "Metadata": { "GODEBUG": "x509negativeserial=1" },
  "Endpoints": {
    "docker": { "Host": "unix:///var/run/docker.sock", "SkipTLSVerify": false }
  }
}

- How I did it

- How to verify it

- Human readable description for the release notes

Allow Docker CLI to set the `GODEBUG` environment variable when the key-value pair (`"GODEBUG":"..."`) exists inside the docker context metadata.

- A picture of a cute animal (not mandatory but encouraged)

codecov-commenter · 2025-08-26T13:25:30Z

Codecov Report

❌ Patch coverage is 55.26316% with 17 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cli/command/cli.go	55.26%	15 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>

thaJeztah · 2025-08-27T11:50:53Z

cli/command/cli.go

+	switch m := meta.Metadata.(type) {
+	case DockerContext:
+		config, ok = m.AdditionalFields[fieldName]
+		if !ok {
+			return
+		}
+	case map[string]any:


Curious if we could even consider just checking for map[string]any - some of the "must match this type" handling with contexts is a bit over-engineered (or at least, ultimately, it's a map[string]any that could be unmarshaled to a custom type).

i tested it and it didn't work 🙈

Ah, right, yeah, I was afraid of that. I think it's because the code already casts the metadata to a specific type. Definitely not critical, so something we can look at later if we want to remove some of those abstractions. Here's (probably) why that happens; the context is set up with a DefaultContextResolver;

cli/cli/command/cli.go

Lines 270 to 272 in b3cd9d4

Resolver: func() (*DefaultContext, error) {

return ResolveDefaultContext(cli.options, *cli.contextStoreConfig)

},

Which I think would convert things;

cli/cli/command/defaultcontextstore.go

Lines 56 to 57 in b3cd9d4

// ResolveDefaultContext creates a Metadata for the current CLI invocation parameters

func ResolveDefaultContext(opts *cliflags.ClientOptions, config store.Config) (*DefaultContext, error) {

thaJeztah · 2025-08-27T11:54:28Z

cli/command/cli.go

+func (cli *DockerCli) setAllowNegativex509() {
+	cn := cli.CurrentContext()
+	meta, err := cli.ContextStore().GetMetadata(cn)


cli.CurrentContext() is just returning the value as-is; no special handling of synchronisation/locking. Given that this is an internal method, we can peel away those layers and just access the field;

cli/cli/command/cli.go

Lines 429 to 431 in e069ded

func (cli *DockerCli) CurrentContext() string {

return cli.currentContext

}

Once we start peeling away those layers, then everything looks to be initialized in the function we're calling this method from, so perhaps the meta could be just passed in as argument (but I haven't looked too closely at that);

cli/cli/command/cli.go

Lines 267 to 273 in 7d7a7aa

cli.currentContext = resolveContextName(cli.options, cli.configFile)

cli.contextStore = &ContextStoreWithDefault{

Store: store.New(config.ContextStoreDir(), *cli.contextStoreConfig),

Resolver: func() (*DefaultContext, error) {

return ResolveDefaultContext(cli.options, *cli.contextStoreConfig)

},

}

The only part that slightly concerns me is if we would be initialising more because of the call to the context store, which may now be called before we even consider making API (or networking) calls. Perhaps not a big issue, but at least from the looks of it, it looks like the "default' (i.e., "no context used") case is still not entirely a "no-op", so we could short-circuit it and return early;

cli/cli/command/defaultcontextstore.go

Lines 135 to 142 in e069ded

func (s *ContextStoreWithDefault) GetMetadata(name string) (store.Metadata, error) {

if name == DefaultContextName {

defaultContext, err := s.Resolver()

if err != nil {

return store.Metadata{}, err

}

return defaultContext.Meta, nil

}

thaJeztah · 2025-08-28T11:51:41Z

Oh; bike-shedding, but I'm considering perhaps we should namespace the option (also in light of having good conventions for these in future if we want to start using the metadata more), e.g.

com.docker.cli.allowx509negativeserial=1

vvoland · 2025-08-29T08:54:14Z

Wondering if we should make it more explicit that x509negativeserial is not an CLI option but a GODEBUG option and we only support the way to propagate it.
Just trying to make sure we don't commit to actually supporting the negative serial behavior working in case Go decides to drop it.

"Metadata": {
  "GODEBUG": {
    "allowx509negativeserialdonotuse": "1"
  }
},

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>

crazy-max · 2025-08-29T10:45:14Z

cli/command/cli.go

+	// set the GODEBUG environment variable with whatever was in the context
+	_ = os.Setenv(fieldName, v)


Wonder if that would override what we currently set in our go.mod on buildx repo: https://github.com/docker/buildx/blob/67218bef58f934f3f19f66346891ea4fbdc1ad4e/go.mod#L187-L188

i think it merges the key-value pairs that are specified by the GODEBUG environment variable with the go.mod ones.

When a GODEBUG setting is not listed in the environment variable, its value is derived from three sources: the defaults for the Go toolchain used to build the program, amended to match the Go version listed in go.mod, and then overridden by explicit //go:debug lines in the program.

https://go.dev/doc/godebug

e.g.

GODEBUG=http2client=0 --- go.mod -- godebug ( http2server=0 )

will result in

godebug ( http2server=0 http2client=0 )

if we specify GODEBUG=winsymlink=1 then it will override the value set inside buildx's go.mod. (but that's already possible without this PR)

Ok looks good thx!

Ah, thanks for checking! I was curious indeed how those options would interact with a go.mod having it set.

Curious; does that go.mod option take effect when building in GOPATH mode? Or does that ignore go.mod entirely?

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>

Copilot

Pull Request Overview

This PR adds an escape hatch mechanism to allow setting the GODEBUG environment variable through Docker context metadata, specifically to support the deprecated x509negativeserial=1 flag for handling certificates with negative serial numbers.

Introduces a setGoDebug function that reads GODEBUG values from context metadata and sets the environment variable
Integrates the function into the CLI initialization process
Adds comprehensive test coverage for the new functionality

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
cli/command/cli.go	Implements the main `setGoDebug` function and integrates it into CLI initialization
cli/command/cli_test.go	Adds unit tests for the new `setGoDebug` functionality

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

cli/command/cli.go

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>

thaJeztah

LGTM

but I need to jump out for some hours, so perhaps @vvoland could double check, and open a cherry-pick 🤗

vvoland

LGTM

Benehiko · 2025-08-29T15:00:32Z

woops, still wanted to squash my commits 🙈

thaJeztah · 2025-08-29T18:35:42Z

Yeah, I missed it this time; well, no harm done!

Add escape hatch for GODEBUG=x509negativeserial

7d7a7aa

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>

Benehiko force-pushed the support-fallback-negative-certs branch from 2f37b74 to 7d7a7aa Compare August 26, 2025 14:07

Benehiko added impact/changelog kind/feature impact/documentation area/context labels Aug 27, 2025

Benehiko requested review from thaJeztah and Copilot August 27, 2025 11:34

This comment was marked as outdated.

Sign in to view

thaJeztah reviewed Aug 27, 2025

View reviewed changes

Benehiko added 2 commits August 29, 2025 12:15

Cleanup setAllowNegativex509

65a6c35

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>

Test setAllowNegativex509

467305f

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>

Benehiko requested a review from Copilot August 29, 2025 10:17

This comment was marked as outdated.

Sign in to view

crazy-max reviewed Aug 29, 2025

View reviewed changes

rename function to fit what it is doing

6163c03

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>

Benehiko marked this pull request as ready for review August 29, 2025 12:09

Benehiko requested review from thaJeztah and Copilot August 29, 2025 12:09

Copilot AI reviewed Aug 29, 2025

View reviewed changes

cli/command/cli.go Show resolved Hide resolved

cli/command/cli.go Show resolved Hide resolved

thaJeztah reviewed Aug 29, 2025

View reviewed changes

cli/command/cli.go Show resolved Hide resolved

return early if GODEBUG set or context is default

72f7933

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>

thaJeztah approved these changes Aug 29, 2025

View reviewed changes

vvoland approved these changes Aug 29, 2025

View reviewed changes

vvoland added this to the 29.0.0 milestone Aug 29, 2025

vvoland merged commit 0155c26 into docker:master Aug 29, 2025
108 of 109 checks passed

vvoland mentioned this pull request Aug 29, 2025

[28.x backport] Add escape hatch for GODEBUG=x509negativeserial #6399

Merged

	Resolver: func() (*DefaultContext, error) {
	return ResolveDefaultContext(cli.options, *cli.contextStoreConfig)
	},

	// ResolveDefaultContext creates a Metadata for the current CLI invocation parameters
	func ResolveDefaultContext(opts cliflags.ClientOptions, config store.Config) (DefaultContext, error) {

	func (cli *DockerCli) CurrentContext() string {
	return cli.currentContext
	}

	cli.currentContext = resolveContextName(cli.options, cli.configFile)
	cli.contextStore = &ContextStoreWithDefault{
	Store: store.New(config.ContextStoreDir(), *cli.contextStoreConfig),
	Resolver: func() (*DefaultContext, error) {
	return ResolveDefaultContext(cli.options, *cli.contextStoreConfig)
	},
	}

	func (s *ContextStoreWithDefault) GetMetadata(name string) (store.Metadata, error) {
	if name == DefaultContextName {
	defaultContext, err := s.Resolver()
	if err != nil {
	return store.Metadata{}, err
	}
	return defaultContext.Meta, nil
	}

		// set the GODEBUG environment variable with whatever was in the context
		_ = os.Setenv(fieldName, v)

Add escape hatch for GODEBUG=x509negativeserial #6371

Add escape hatch for GODEBUG=x509negativeserial #6371

Uh oh!

Conversation

Benehiko commented Aug 26, 2025 • edited by vvoland Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Aug 28, 2025

Uh oh!

vvoland commented Aug 29, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

vvoland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Benehiko commented Aug 29, 2025

Uh oh!

thaJeztah commented Aug 29, 2025

Uh oh!

Uh oh!

Benehiko commented Aug 26, 2025 •

edited by vvoland

Loading

codecov-commenter commented Aug 26, 2025 •

edited

Loading