diff --git a/client/client.go b/client/client.go index 5306e5b091..dd5f060b0a 100644 --- a/client/client.go +++ b/client/client.go @@ -212,9 +212,9 @@ func New(ops ...Opt) (*Client, error) { } if cfg.envAPIVersion != "" { - cfg.version = cfg.envAPIVersion + c.setAPIVersion(cfg.envAPIVersion) } else if cfg.manualAPIVersion != "" { - cfg.version = cfg.manualAPIVersion + c.setAPIVersion(cfg.manualAPIVersion) } if tr, ok := c.client.Transport.(*http.Transport); ok { @@ -282,7 +282,7 @@ func (cli *Client) Close() error { // be negotiated when making the actual requests, and for which cases // we cannot do the negotiation lazily. func (cli *Client) checkVersion(ctx context.Context) error { - if cli.manualOverride || !cli.negotiateVersion || cli.negotiated.Load() { + if cli.negotiated.Load() || !cli.negotiateVersion { return nil } _, err := cli.Ping(ctx, PingOptions{ @@ -311,15 +311,16 @@ func (cli *Client) ClientVersion() string { // negotiateAPIVersion updates the version to match the API version from // the ping response. It falls back to the lowest version supported if the -// API version is empty, or returns an error if the API version is lower than -// the lowest supported API version, in which case the version is not modified. +// API version is empty. +// +// It returns an error if version is invalid, or lower than the minimum +// supported API version in which case the client's API version is not +// updated, and negotiation is not marked as completed. func (cli *Client) negotiateAPIVersion(pingVersion string) error { pingVersion = strings.TrimPrefix(pingVersion, "v") if pingVersion == "" { // TODO(thaJeztah): consider returning an error on empty value or not falling back; see https://github.com/moby/moby/pull/51119#discussion_r2413148487 pingVersion = MinAPIVersion - } else if versions.LessThan(pingVersion, MinAPIVersion) { - return cerrdefs.ErrInvalidArgument.WithMessage(fmt.Sprintf("API version %s is not supported by this client: the minimum supported API version is %s", pingVersion, MinAPIVersion)) } var err error @@ -328,24 +329,35 @@ func (cli *Client) negotiateAPIVersion(pingVersion string) error { return err } + if versions.LessThan(pingVersion, MinAPIVersion) { + return cerrdefs.ErrInvalidArgument.WithMessage(fmt.Sprintf("API version %s is not supported by this client: the minimum supported API version is %s", pingVersion, MinAPIVersion)) + } + // if the client is not initialized with a version, start with the latest supported version - if cli.version == "" { - cli.version = MaxAPIVersion + negotiatedVersion := cli.version + if negotiatedVersion == "" { + negotiatedVersion = MaxAPIVersion } // if server version is lower than the client version, downgrade - if versions.LessThan(pingVersion, cli.version) { - cli.version = pingVersion + if versions.LessThan(pingVersion, negotiatedVersion) { + negotiatedVersion = pingVersion } // Store the results, so that automatic API version negotiation (if enabled) // won't be performed on the next request. - if cli.negotiateVersion { - cli.negotiated.Store(true) - } + cli.setAPIVersion(negotiatedVersion) return nil } +// setAPIVersion sets the client's API version and marks API version negotiation +// as completed, so that automatic API version negotiation (if enabled) won't +// be performed on the next request. +func (cli *Client) setAPIVersion(version string) { + cli.version = version + cli.negotiated.Store(true) +} + // DaemonHost returns the host address used by the client func (cli *Client) DaemonHost() string { return cli.host diff --git a/client/client_options.go b/client/client_options.go index 81b1e9797d..7236f88638 100644 --- a/client/client_options.go +++ b/client/client_options.go @@ -38,8 +38,6 @@ type clientConfig struct { userAgent *string // custom HTTP headers configured by users. customHTTPHeaders map[string]string - // manualOverride is set to true when the version was set by users. - manualOverride bool // manualAPIVersion contains the API version set by users. This field // will only be non-empty if a valid-formed version was set through @@ -278,7 +276,6 @@ func WithAPIVersion(version string) Opt { return fmt.Errorf("invalid API version (%s): %w", version, err) } c.manualAPIVersion = ver - c.manualOverride = true } return nil } @@ -311,7 +308,6 @@ func WithAPIVersionFromEnv() Opt { return fmt.Errorf("invalid API version (%s): %w", version, err) } c.envAPIVersion = ver - c.manualOverride = true } return nil } diff --git a/client/client_options_test.go b/client/client_options_test.go index 319e65c207..7271926578 100644 --- a/client/client_options_test.go +++ b/client/client_options_test.go @@ -141,7 +141,7 @@ func TestOptionWithAPIVersion(t *testing.T) { assert.Check(t, client != nil) assert.Check(t, is.Equal(client.ClientVersion(), tc.expected)) isNoOp := strings.TrimPrefix(strings.TrimSpace(tc.version), "v") == "" - assert.Check(t, is.Equal(client.manualOverride, !isNoOp)) + assert.Check(t, is.Equal(client.negotiated.Load(), !isNoOp)) } }) } @@ -244,7 +244,7 @@ func TestOptionWithAPIVersionFromEnv(t *testing.T) { assert.Check(t, client != nil) assert.Check(t, is.Equal(client.ClientVersion(), tc.expected)) isNoOp := strings.TrimPrefix(strings.TrimSpace(tc.version), "v") == "" - assert.Check(t, is.Equal(client.manualOverride, !isNoOp)) + assert.Check(t, is.Equal(client.negotiated.Load(), !isNoOp)) } }) } @@ -258,7 +258,7 @@ func TestOptionOverridePriority(t *testing.T) { client, err := New(WithAPIVersionFromEnv(), WithAPIVersion("1.50")) assert.NilError(t, err) assert.Check(t, is.Equal(client.ClientVersion(), "1.50")) - assert.Check(t, is.Equal(client.manualOverride, true)) + assert.Check(t, is.Equal(client.negotiated.Load(), true)) }) const expected = "1.51" @@ -268,28 +268,28 @@ func TestOptionOverridePriority(t *testing.T) { client, err := New(WithAPIVersionFromEnv(), WithAPIVersion("1.50")) assert.NilError(t, err) assert.Check(t, is.Equal(client.ClientVersion(), expected)) - assert.Check(t, is.Equal(client.manualOverride, true)) + assert.Check(t, is.Equal(client.negotiated.Load(), true)) }) t.Run("WithAPIVersionFromEnv last", func(t *testing.T) { client, err := New(WithAPIVersion("1.50"), WithAPIVersionFromEnv()) assert.NilError(t, err) assert.Check(t, is.Equal(client.ClientVersion(), expected)) - assert.Check(t, is.Equal(client.manualOverride, true)) + assert.Check(t, is.Equal(client.negotiated.Load(), true)) }) t.Run("FromEnv first", func(t *testing.T) { client, err := New(FromEnv, WithAPIVersion("1.50")) assert.NilError(t, err) assert.Check(t, is.Equal(client.ClientVersion(), expected)) - assert.Check(t, is.Equal(client.manualOverride, true)) + assert.Check(t, is.Equal(client.negotiated.Load(), true)) }) t.Run("FromEnv last", func(t *testing.T) { client, err := New(WithAPIVersion("1.50"), FromEnv) assert.NilError(t, err) assert.Check(t, is.Equal(client.ClientVersion(), expected)) - assert.Check(t, is.Equal(client.manualOverride, true)) + assert.Check(t, is.Equal(client.negotiated.Load(), true)) }) } diff --git a/client/ping.go b/client/ping.go index 3238f29567..3f001ecf73 100644 --- a/client/ping.go +++ b/client/ping.go @@ -71,7 +71,7 @@ type SwarmStatus struct { // for other non-success status codes, failing to connect to the API, or failing // to parse the API response. func (cli *Client) Ping(ctx context.Context, options PingOptions) (PingResult, error) { - if (cli.manualOverride || cli.negotiated.Load()) && !options.ForceNegotiate { + if cli.negotiated.Load() && !options.ForceNegotiate { // API version was already negotiated or manually set. return cli.ping(ctx) } diff --git a/vendor/github.com/moby/moby/client/client.go b/vendor/github.com/moby/moby/client/client.go index 5306e5b091..dd5f060b0a 100644 --- a/vendor/github.com/moby/moby/client/client.go +++ b/vendor/github.com/moby/moby/client/client.go @@ -212,9 +212,9 @@ func New(ops ...Opt) (*Client, error) { } if cfg.envAPIVersion != "" { - cfg.version = cfg.envAPIVersion + c.setAPIVersion(cfg.envAPIVersion) } else if cfg.manualAPIVersion != "" { - cfg.version = cfg.manualAPIVersion + c.setAPIVersion(cfg.manualAPIVersion) } if tr, ok := c.client.Transport.(*http.Transport); ok { @@ -282,7 +282,7 @@ func (cli *Client) Close() error { // be negotiated when making the actual requests, and for which cases // we cannot do the negotiation lazily. func (cli *Client) checkVersion(ctx context.Context) error { - if cli.manualOverride || !cli.negotiateVersion || cli.negotiated.Load() { + if cli.negotiated.Load() || !cli.negotiateVersion { return nil } _, err := cli.Ping(ctx, PingOptions{ @@ -311,15 +311,16 @@ func (cli *Client) ClientVersion() string { // negotiateAPIVersion updates the version to match the API version from // the ping response. It falls back to the lowest version supported if the -// API version is empty, or returns an error if the API version is lower than -// the lowest supported API version, in which case the version is not modified. +// API version is empty. +// +// It returns an error if version is invalid, or lower than the minimum +// supported API version in which case the client's API version is not +// updated, and negotiation is not marked as completed. func (cli *Client) negotiateAPIVersion(pingVersion string) error { pingVersion = strings.TrimPrefix(pingVersion, "v") if pingVersion == "" { // TODO(thaJeztah): consider returning an error on empty value or not falling back; see https://github.com/moby/moby/pull/51119#discussion_r2413148487 pingVersion = MinAPIVersion - } else if versions.LessThan(pingVersion, MinAPIVersion) { - return cerrdefs.ErrInvalidArgument.WithMessage(fmt.Sprintf("API version %s is not supported by this client: the minimum supported API version is %s", pingVersion, MinAPIVersion)) } var err error @@ -328,24 +329,35 @@ func (cli *Client) negotiateAPIVersion(pingVersion string) error { return err } + if versions.LessThan(pingVersion, MinAPIVersion) { + return cerrdefs.ErrInvalidArgument.WithMessage(fmt.Sprintf("API version %s is not supported by this client: the minimum supported API version is %s", pingVersion, MinAPIVersion)) + } + // if the client is not initialized with a version, start with the latest supported version - if cli.version == "" { - cli.version = MaxAPIVersion + negotiatedVersion := cli.version + if negotiatedVersion == "" { + negotiatedVersion = MaxAPIVersion } // if server version is lower than the client version, downgrade - if versions.LessThan(pingVersion, cli.version) { - cli.version = pingVersion + if versions.LessThan(pingVersion, negotiatedVersion) { + negotiatedVersion = pingVersion } // Store the results, so that automatic API version negotiation (if enabled) // won't be performed on the next request. - if cli.negotiateVersion { - cli.negotiated.Store(true) - } + cli.setAPIVersion(negotiatedVersion) return nil } +// setAPIVersion sets the client's API version and marks API version negotiation +// as completed, so that automatic API version negotiation (if enabled) won't +// be performed on the next request. +func (cli *Client) setAPIVersion(version string) { + cli.version = version + cli.negotiated.Store(true) +} + // DaemonHost returns the host address used by the client func (cli *Client) DaemonHost() string { return cli.host diff --git a/vendor/github.com/moby/moby/client/client_options.go b/vendor/github.com/moby/moby/client/client_options.go index 81b1e9797d..7236f88638 100644 --- a/vendor/github.com/moby/moby/client/client_options.go +++ b/vendor/github.com/moby/moby/client/client_options.go @@ -38,8 +38,6 @@ type clientConfig struct { userAgent *string // custom HTTP headers configured by users. customHTTPHeaders map[string]string - // manualOverride is set to true when the version was set by users. - manualOverride bool // manualAPIVersion contains the API version set by users. This field // will only be non-empty if a valid-formed version was set through @@ -278,7 +276,6 @@ func WithAPIVersion(version string) Opt { return fmt.Errorf("invalid API version (%s): %w", version, err) } c.manualAPIVersion = ver - c.manualOverride = true } return nil } @@ -311,7 +308,6 @@ func WithAPIVersionFromEnv() Opt { return fmt.Errorf("invalid API version (%s): %w", version, err) } c.envAPIVersion = ver - c.manualOverride = true } return nil } diff --git a/vendor/github.com/moby/moby/client/ping.go b/vendor/github.com/moby/moby/client/ping.go index 3238f29567..3f001ecf73 100644 --- a/vendor/github.com/moby/moby/client/ping.go +++ b/vendor/github.com/moby/moby/client/ping.go @@ -71,7 +71,7 @@ type SwarmStatus struct { // for other non-success status codes, failing to connect to the API, or failing // to parse the API response. func (cli *Client) Ping(ctx context.Context, options PingOptions) (PingResult, error) { - if (cli.manualOverride || cli.negotiated.Load()) && !options.ForceNegotiate { + if cli.negotiated.Load() && !options.ForceNegotiate { // API version was already negotiated or manually set. return cli.ping(ctx) }