From a0385bf042f09010f3c3c2bfd944838d961979d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Wed, 9 Apr 2025 14:13:59 +0200 Subject: [PATCH 1/2] swarm/init: Test `init --external-ca` with custom cert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Gronowski --- cli/command/swarm/client_test.go | 6 ++--- cli/command/swarm/init_test.go | 40 +++++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/cli/command/swarm/client_test.go b/cli/command/swarm/client_test.go index 9e37db0e60..5b4a0aa32e 100644 --- a/cli/command/swarm/client_test.go +++ b/cli/command/swarm/client_test.go @@ -12,7 +12,7 @@ import ( type fakeClient struct { client.Client infoFunc func() (system.Info, error) - swarmInitFunc func() (string, error) + swarmInitFunc func(req swarm.InitRequest) (string, error) swarmInspectFunc func() (swarm.Swarm, error) nodeInspectFunc func() (swarm.Node, []byte, error) swarmGetUnlockKeyFunc func() (types.SwarmUnlockKeyResponse, error) @@ -36,9 +36,9 @@ func (cli *fakeClient) NodeInspectWithRaw(context.Context, string) (swarm.Node, return swarm.Node{}, []byte{}, nil } -func (cli *fakeClient) SwarmInit(context.Context, swarm.InitRequest) (string, error) { +func (cli *fakeClient) SwarmInit(_ context.Context, req swarm.InitRequest) (string, error) { if cli.swarmInitFunc != nil { - return cli.swarmInitFunc() + return cli.swarmInitFunc(req) } return "", nil } diff --git a/cli/command/swarm/init_test.go b/cli/command/swarm/init_test.go index f76ae79c66..0d62cd7d56 100644 --- a/cli/command/swarm/init_test.go +++ b/cli/command/swarm/init_test.go @@ -4,12 +4,15 @@ import ( "errors" "fmt" "io" + "os" + "path/filepath" "testing" "github.com/docker/cli/internal/test" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/swarm" "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/golden" ) @@ -17,7 +20,7 @@ func TestSwarmInitErrorOnAPIFailure(t *testing.T) { testCases := []struct { name string flags map[string]string - swarmInitFunc func() (string, error) + swarmInitFunc func(swarm.InitRequest) (string, error) swarmInspectFunc func() (swarm.Swarm, error) swarmGetUnlockKeyFunc func() (types.SwarmUnlockKeyResponse, error) nodeInspectFunc func() (swarm.Node, []byte, error) @@ -25,14 +28,14 @@ func TestSwarmInitErrorOnAPIFailure(t *testing.T) { }{ { name: "init-failed", - swarmInitFunc: func() (string, error) { + swarmInitFunc: func(swarm.InitRequest) (string, error) { return "", errors.New("error initializing the swarm") }, expectedError: "error initializing the swarm", }, { name: "init-failed-with-ip-choice", - swarmInitFunc: func() (string, error) { + swarmInitFunc: func(swarm.InitRequest) (string, error) { return "", errors.New("could not choose an IP address to advertise") }, expectedError: "could not choose an IP address to advertise - specify one with --advertise-addr", @@ -86,14 +89,14 @@ func TestSwarmInit(t *testing.T) { testCases := []struct { name string flags map[string]string - swarmInitFunc func() (string, error) + swarmInitFunc func(req swarm.InitRequest) (string, error) swarmInspectFunc func() (swarm.Swarm, error) swarmGetUnlockKeyFunc func() (types.SwarmUnlockKeyResponse, error) nodeInspectFunc func() (swarm.Node, []byte, error) }{ { name: "init", - swarmInitFunc: func() (string, error) { + swarmInitFunc: func(swarm.InitRequest) (string, error) { return "nodeID", nil }, }, @@ -102,7 +105,7 @@ func TestSwarmInit(t *testing.T) { flags: map[string]string{ flagAutolock: "true", }, - swarmInitFunc: func() (string, error) { + swarmInitFunc: func(swarm.InitRequest) (string, error) { return "nodeID", nil }, swarmGetUnlockKeyFunc: func() (types.SwarmUnlockKeyResponse, error) { @@ -132,3 +135,28 @@ func TestSwarmInit(t *testing.T) { }) } } + +func TestSwarmInitWithExternalCA(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{ + swarmInitFunc: func(req swarm.InitRequest) (string, error) { + if assert.Check(t, is.Len(req.Spec.CAConfig.ExternalCAs, 1)) { + assert.Equal(t, req.Spec.CAConfig.ExternalCAs[0].CACert, cert) + assert.Equal(t, req.Spec.CAConfig.ExternalCAs[0].Protocol, swarm.ExternalCAProtocolCFSSL) + assert.Equal(t, req.Spec.CAConfig.ExternalCAs[0].URL, "https://example.com") + } + return "nodeID", nil + }, + }) + + tempDir := t.TempDir() + certFile := filepath.Join(tempDir, "cert.pem") + err := os.WriteFile(certFile, []byte(cert), 0644) + assert.NilError(t, err) + + cmd := newInitCommand(cli) + cmd.SetArgs([]string{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + assert.NilError(t, cmd.Flags().Set(flagExternalCA, "protocol=cfssl,url=https://example.com,cacert="+certFile)) + assert.NilError(t, cmd.Execute()) +} From 6c2d023d87374f5e8655004da217a23d5deb609f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Wed, 9 Apr 2025 14:14:09 +0200 Subject: [PATCH 2/2] swarm/init: Fix `--external-ca` ignoring `cacert` option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 31d629245855d54294e04c6e7202f21901da5310 mistakenly changed the `ToSpec` function to set all certs passed via `external-ca` to empty strings. Signed-off-by: Paweł Gronowski --- cli/command/swarm/ca.go | 2 +- cli/command/swarm/init_test.go | 2 +- cli/command/swarm/opts.go | 14 +++++++++----- cli/command/swarm/update.go | 2 +- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/cli/command/swarm/ca.go b/cli/command/swarm/ca.go index 59974ac644..afd2b047bb 100644 --- a/cli/command/swarm/ca.go +++ b/cli/command/swarm/ca.go @@ -96,7 +96,7 @@ func runCA(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, opt func updateSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet, opts caOptions) { caCert := opts.rootCACert.Contents() caKey := opts.rootCAKey.Contents() - opts.mergeSwarmSpecCAFlags(spec, flags, caCert) + opts.mergeSwarmSpecCAFlags(spec, flags, &caCert) spec.CAConfig.SigningCACert = caCert spec.CAConfig.SigningCAKey = caKey diff --git a/cli/command/swarm/init_test.go b/cli/command/swarm/init_test.go index 0d62cd7d56..b0e2decf0e 100644 --- a/cli/command/swarm/init_test.go +++ b/cli/command/swarm/init_test.go @@ -150,7 +150,7 @@ func TestSwarmInitWithExternalCA(t *testing.T) { tempDir := t.TempDir() certFile := filepath.Join(tempDir, "cert.pem") - err := os.WriteFile(certFile, []byte(cert), 0644) + err := os.WriteFile(certFile, []byte(cert), 0o644) assert.NilError(t, err) cmd := newInitCommand(cli) diff --git a/cli/command/swarm/opts.go b/cli/command/swarm/opts.go index a29a67147c..41a0244ed4 100644 --- a/cli/command/swarm/opts.go +++ b/cli/command/swarm/opts.go @@ -231,7 +231,7 @@ func addSwarmFlags(flags *pflag.FlagSet, options *swarmOptions) { addSwarmCAFlags(flags, &options.swarmCAOptions) } -func (o *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet, caCert string) { +func (o *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet, caCert *string) { if flags.Changed(flagTaskHistoryLimit) { spec.Orchestration.TaskHistoryRetentionLimit = &o.taskHistoryLimit } @@ -255,20 +255,24 @@ type swarmCAOptions struct { externalCA ExternalCAOption } -func (o *swarmCAOptions) mergeSwarmSpecCAFlags(spec *swarm.Spec, flags *pflag.FlagSet, caCert string) { +func (o *swarmCAOptions) mergeSwarmSpecCAFlags(spec *swarm.Spec, flags *pflag.FlagSet, caCert *string) { if flags.Changed(flagCertExpiry) { spec.CAConfig.NodeCertExpiry = o.nodeCertExpiry } if flags.Changed(flagExternalCA) { spec.CAConfig.ExternalCAs = o.externalCA.Value() - for _, ca := range spec.CAConfig.ExternalCAs { - ca.CACert = caCert + if caCert != nil { + for _, ca := range spec.CAConfig.ExternalCAs { + if ca.CACert == "" { + ca.CACert = *caCert + } + } } } } func (o *swarmOptions) ToSpec(flags *pflag.FlagSet) swarm.Spec { var spec swarm.Spec - o.mergeSwarmSpec(&spec, flags, "") + o.mergeSwarmSpec(&spec, flags, nil) return spec } diff --git a/cli/command/swarm/update.go b/cli/command/swarm/update.go index 50ddc17b1a..2e853a9312 100644 --- a/cli/command/swarm/update.go +++ b/cli/command/swarm/update.go @@ -53,7 +53,7 @@ func runUpdate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, prevAutoLock := swarmInspect.Spec.EncryptionConfig.AutoLockManagers - opts.mergeSwarmSpec(&swarmInspect.Spec, flags, swarmInspect.ClusterInfo.TLSInfo.TrustRoot) + opts.mergeSwarmSpec(&swarmInspect.Spec, flags, &swarmInspect.ClusterInfo.TLSInfo.TrustRoot) curAutoLock := swarmInspect.Spec.EncryptionConfig.AutoLockManagers