From e86ea2290073d6dccb0d8295f31299673c219a8b Mon Sep 17 00:00:00 2001 From: Devon Steenberg Date: Mon, 25 Aug 2025 20:25:07 +1200 Subject: [PATCH] fix(sslflags): Deprecate ssl flags [BE-12168] (#1076) --- api/cli/cli.go | 77 +++++++++++++--- api/cli/cli_test.go | 185 ++++++++++++++++++++++++++++++++++++++ api/cli/pairlist.go | 2 +- api/cmd/portainer/main.go | 2 +- api/portainer.go | 4 +- go.mod | 3 +- go.sum | 5 +- 7 files changed, 256 insertions(+), 22 deletions(-) diff --git a/api/cli/cli.go b/api/cli/cli.go index 2ab93d36f..267f584d9 100644 --- a/api/cli/cli.go +++ b/api/cli/cli.go @@ -9,8 +9,8 @@ import ( portainer "github.com/portainer/portainer/api" + "github.com/alecthomas/kingpin/v2" "github.com/rs/zerolog/log" - "gopkg.in/alecthomas/kingpin.v2" ) // Service implements the CLIService interface @@ -35,16 +35,9 @@ func CLIFlags() *portainer.CLIFlags { FeatureFlags: kingpin.Flag("feat", "List of feature flags").Strings(), EnableEdgeComputeFeatures: kingpin.Flag("edge-compute", "Enable Edge Compute features").Bool(), NoAnalytics: kingpin.Flag("no-analytics", "Disable Analytics in app (deprecated)").Bool(), - TLS: kingpin.Flag("tlsverify", "TLS support").Default(defaultTLS).Bool(), TLSSkipVerify: kingpin.Flag("tlsskipverify", "Disable TLS server verification").Default(defaultTLSSkipVerify).Bool(), - TLSCacert: kingpin.Flag("tlscacert", "Path to the CA").Default(defaultTLSCACertPath).String(), - TLSCert: kingpin.Flag("tlscert", "Path to the TLS certificate file").Default(defaultTLSCertPath).String(), - TLSKey: kingpin.Flag("tlskey", "Path to the TLS key").Default(defaultTLSKeyPath).String(), HTTPDisabled: kingpin.Flag("http-disabled", "Serve portainer only on https").Default(defaultHTTPDisabled).Bool(), HTTPEnabled: kingpin.Flag("http-enabled", "Serve portainer on http").Default(defaultHTTPEnabled).Bool(), - SSL: kingpin.Flag("ssl", "Secure Portainer instance using SSL (deprecated)").Default(defaultSSL).Bool(), - SSLCert: kingpin.Flag("sslcert", "Path to the SSL certificate used to secure the Portainer instance").String(), - SSLKey: kingpin.Flag("sslkey", "Path to the SSL key used to secure the Portainer instance").String(), Rollback: kingpin.Flag("rollback", "Rollback the database to the previous backup").Bool(), SnapshotInterval: kingpin.Flag("snapshot-interval", "Duration between each environment snapshot job").String(), AdminPassword: kingpin.Flag("admin-password", "Set admin password with provided hash").String(), @@ -70,8 +63,37 @@ func CLIFlags() *portainer.CLIFlags { func (Service) ParseFlags(version string) (*portainer.CLIFlags, error) { kingpin.Version(version) + var hasSSLFlag, hasSSLCertFlag, hasSSLKeyFlag bool + sslFlag := kingpin.Flag( + "ssl", + "Secure Portainer instance using SSL (deprecated)", + ).Default(defaultSSL).IsSetByUser(&hasSSLFlag) + ssl := sslFlag.Bool() + sslCertFlag := kingpin.Flag( + "sslcert", + "Path to the SSL certificate used to secure the Portainer instance", + ).IsSetByUser(&hasSSLCertFlag) + sslCert := sslCertFlag.String() + sslKeyFlag := kingpin.Flag( + "sslkey", + "Path to the SSL key used to secure the Portainer instance", + ).IsSetByUser(&hasSSLKeyFlag) + sslKey := sslKeyFlag.String() + flags := CLIFlags() + var hasTLSFlag, hasTLSCertFlag, hasTLSKeyFlag bool + tlsFlag := kingpin.Flag("tlsverify", "TLS support").Default(defaultTLS).IsSetByUser(&hasTLSFlag) + flags.TLS = tlsFlag.Bool() + tlsCertFlag := kingpin.Flag( + "tlscert", + "Path to the TLS certificate file", + ).Default(defaultTLSCertPath).IsSetByUser(&hasTLSCertFlag) + flags.TLSCert = tlsCertFlag.String() + tlsKeyFlag := kingpin.Flag("tlskey", "Path to the TLS key").Default(defaultTLSKeyPath).IsSetByUser(&hasTLSKeyFlag) + flags.TLSKey = tlsKeyFlag.String() + flags.TLSCacert = kingpin.Flag("tlscacert", "Path to the CA").Default(defaultTLSCACertPath).String() + kingpin.Parse() if !filepath.IsAbs(*flags.Assets) { @@ -83,6 +105,41 @@ func (Service) ParseFlags(version string) (*portainer.CLIFlags, error) { *flags.Assets = filepath.Join(filepath.Dir(ex), *flags.Assets) } + // If the user didn't provide a tls flag remove the defaults to match previous behaviour + if !hasTLSFlag { + if !hasTLSCertFlag { + *flags.TLSCert = "" + } + + if !hasTLSKeyFlag { + *flags.TLSKey = "" + } + } + + if hasSSLFlag { + log.Warn().Msgf("the %q flag is deprecated. use %q instead.", sslFlag.Model().Name, tlsFlag.Model().Name) + + if !hasTLSFlag { + flags.TLS = ssl + } + } + + if hasSSLCertFlag { + log.Warn().Msgf("the %q flag is deprecated. use %q instead.", sslCertFlag.Model().Name, tlsCertFlag.Model().Name) + + if !hasTLSCertFlag { + flags.TLSCert = sslCert + } + } + + if hasSSLKeyFlag { + log.Warn().Msgf("the %q flag is deprecated. use %q instead.", sslKeyFlag.Model().Name, tlsKeyFlag.Model().Name) + + if !hasTLSKeyFlag { + flags.TLSKey = sslKey + } + } + return flags, nil } @@ -109,10 +166,6 @@ func displayDeprecationWarnings(flags *portainer.CLIFlags) { if *flags.NoAnalytics { log.Warn().Msg("the --no-analytics flag has been kept to allow migration of instances running a previous version of Portainer with this flag enabled, to version 2.0 where enabling this flag will have no effect") } - - if *flags.SSL { - log.Warn().Msg("SSL is enabled by default and there is no need for the --ssl flag, it has been kept to allow migration of instances running a previous version of Portainer with this flag enabled") - } } func validateEndpointURL(endpointURL string) error { diff --git a/api/cli/cli_test.go b/api/cli/cli_test.go index 82f02a0e4..08a9fbb4b 100644 --- a/api/cli/cli_test.go +++ b/api/cli/cli_test.go @@ -1,9 +1,12 @@ package cli import ( + "io" "os" + "strings" "testing" + zerolog "github.com/rs/zerolog/log" "github.com/stretchr/testify/require" ) @@ -22,3 +25,185 @@ func TestOptionParser(t *testing.T) { require.False(t, *opts.HTTPDisabled) require.True(t, *opts.EnableEdgeComputeFeatures) } + +func TestParseTLSFlags(t *testing.T) { + testCases := []struct { + name string + args []string + expectedTLSFlag bool + expectedTLSCertFlag string + expectedTLSKeyFlag string + expectedLogMessages []string + }{ + { + name: "no flags", + expectedTLSFlag: false, + expectedTLSCertFlag: "", + expectedTLSKeyFlag: "", + }, + { + name: "only ssl flag", + args: []string{ + "portainer", + "--ssl", + }, + expectedTLSFlag: true, + expectedTLSCertFlag: "", + expectedTLSKeyFlag: "", + }, + { + name: "only tls flag", + args: []string{ + "portainer", + "--tlsverify", + }, + expectedTLSFlag: true, + expectedTLSCertFlag: defaultTLSCertPath, + expectedTLSKeyFlag: defaultTLSKeyPath, + }, + { + name: "partial ssl flags", + args: []string{ + "portainer", + "--ssl", + "--sslcert=ssl-cert-flag-value", + }, + expectedTLSFlag: true, + expectedTLSCertFlag: "ssl-cert-flag-value", + expectedTLSKeyFlag: "", + }, + { + name: "partial tls flags", + args: []string{ + "portainer", + "--tlsverify", + "--tlscert=tls-cert-flag-value", + }, + expectedTLSFlag: true, + expectedTLSCertFlag: "tls-cert-flag-value", + expectedTLSKeyFlag: defaultTLSKeyPath, + }, + { + name: "partial tls and ssl flags", + args: []string{ + "portainer", + "--tlsverify", + "--tlscert=tls-cert-flag-value", + "--sslkey=ssl-key-flag-value", + }, + expectedTLSFlag: true, + expectedTLSCertFlag: "tls-cert-flag-value", + expectedTLSKeyFlag: "ssl-key-flag-value", + }, + { + name: "partial tls and ssl flags 2", + args: []string{ + "portainer", + "--ssl", + "--tlscert=tls-cert-flag-value", + "--sslkey=ssl-key-flag-value", + }, + expectedTLSFlag: true, + expectedTLSCertFlag: "tls-cert-flag-value", + expectedTLSKeyFlag: "ssl-key-flag-value", + }, + { + name: "ssl flags", + args: []string{ + "portainer", + "--ssl", + "--sslcert=ssl-cert-flag-value", + "--sslkey=ssl-key-flag-value", + }, + expectedTLSFlag: true, + expectedTLSCertFlag: "ssl-cert-flag-value", + expectedTLSKeyFlag: "ssl-key-flag-value", + expectedLogMessages: []string{ + "the \\\"ssl\\\" flag is deprecated. use \\\"tlsverify\\\" instead.", + "the \\\"sslcert\\\" flag is deprecated. use \\\"tlscert\\\" instead.", + "the \\\"sslkey\\\" flag is deprecated. use \\\"tlskey\\\" instead.", + }, + }, + { + name: "tls flags", + args: []string{ + "portainer", + "--tlsverify", + "--tlscert=tls-cert-flag-value", + "--tlskey=tls-key-flag-value", + }, + expectedTLSFlag: true, + expectedTLSCertFlag: "tls-cert-flag-value", + expectedTLSKeyFlag: "tls-key-flag-value", + }, + { + name: "tls and ssl flags", + args: []string{ + "portainer", + "--tlsverify", + "--tlscert=tls-cert-flag-value", + "--tlskey=tls-key-flag-value", + "--ssl", + "--sslcert=ssl-cert-flag-value", + "--sslkey=ssl-key-flag-value", + }, + expectedTLSFlag: true, + expectedTLSCertFlag: "tls-cert-flag-value", + expectedTLSKeyFlag: "tls-key-flag-value", + expectedLogMessages: []string{ + "the \\\"ssl\\\" flag is deprecated. use \\\"tlsverify\\\" instead.", + "the \\\"sslcert\\\" flag is deprecated. use \\\"tlscert\\\" instead.", + "the \\\"sslkey\\\" flag is deprecated. use \\\"tlskey\\\" instead.", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var logOutput strings.Builder + setupLogOutput(t, &logOutput) + + if tc.args == nil { + tc.args = []string{"portainer"} + } + setOsArgs(t, tc.args) + + s := Service{} + flags, err := s.ParseFlags("test-version") + if err != nil { + t.Fatalf("error parsing flags: %v", err) + } + + if flags.TLS == nil { + t.Fatal("TLS flag was nil") + } + + require.Equal(t, tc.expectedTLSFlag, *flags.TLS, "tlsverify flag didn't match") + require.Equal(t, tc.expectedTLSCertFlag, *flags.TLSCert, "tlscert flag didn't match") + require.Equal(t, tc.expectedTLSKeyFlag, *flags.TLSKey, "tlskey flag didn't match") + + for _, expectedLogMessage := range tc.expectedLogMessages { + require.Contains(t, logOutput.String(), expectedLogMessage, "Log didn't contain expected message") + } + }) + } +} + +func setOsArgs(t *testing.T, args []string) { + t.Helper() + previousArgs := os.Args + os.Args = args + t.Cleanup(func() { + os.Args = previousArgs + }) +} + +func setupLogOutput(t *testing.T, w io.Writer) { + t.Helper() + + oldLogger := zerolog.Logger + zerolog.Logger = zerolog.Output(w) + t.Cleanup(func() { + zerolog.Logger = oldLogger + }) +} diff --git a/api/cli/pairlist.go b/api/cli/pairlist.go index 2d42d450f..ded08bf56 100644 --- a/api/cli/pairlist.go +++ b/api/cli/pairlist.go @@ -6,7 +6,7 @@ import ( "fmt" "strings" - "gopkg.in/alecthomas/kingpin.v2" + "github.com/alecthomas/kingpin/v2" ) type pairList []portainer.Pair diff --git a/api/cmd/portainer/main.go b/api/cmd/portainer/main.go index 6ad878db6..fcc4a7579 100644 --- a/api/cmd/portainer/main.go +++ b/api/cmd/portainer/main.go @@ -408,7 +408,7 @@ func buildServer(flags *portainer.CLIFlags) portainer.Server { edgeStacksService := edgestacks.NewService(dataStore) - sslService, err := initSSLService(*flags.AddrHTTPS, *flags.SSLCert, *flags.SSLKey, fileService, dataStore, shutdownTrigger) + sslService, err := initSSLService(*flags.AddrHTTPS, *flags.TLSCert, *flags.TLSKey, fileService, dataStore, shutdownTrigger) if err != nil { log.Fatal().Err(err).Msg("") } diff --git a/api/portainer.go b/api/portainer.go index 5a45ad136..0848152bd 100644 --- a/api/portainer.go +++ b/api/portainer.go @@ -122,14 +122,12 @@ type ( Templates *string TLS *bool TLSSkipVerify *bool + HasTLSCacert *bool TLSCacert *string TLSCert *string TLSKey *string HTTPDisabled *bool HTTPEnabled *bool - SSL *bool - SSLCert *string - SSLKey *string Rollback *bool SnapshotInterval *string BaseURL *string diff --git a/go.mod b/go.mod index 2381c448f..e831e9fec 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/Microsoft/go-winio v0.6.2 github.com/RoaringBitmap/roaring/v2 v2.5.0 github.com/VictoriaMetrics/fastcache v1.12.0 + github.com/alecthomas/kingpin/v2 v2.4.0 github.com/aws/aws-sdk-go-v2 v1.30.3 github.com/aws/aws-sdk-go-v2/credentials v1.17.27 github.com/aws/aws-sdk-go-v2/service/ecr v1.24.1 @@ -54,7 +55,6 @@ require ( golang.org/x/mod v0.25.0 golang.org/x/oauth2 v0.29.0 golang.org/x/sync v0.16.0 - gopkg.in/alecthomas/kingpin.v2 v2.2.6 gopkg.in/yaml.v2 v2.4.0 gopkg.in/yaml.v3 v3.0.1 helm.sh/helm/v3 v3.18.5 @@ -86,7 +86,6 @@ require ( github.com/ProtonMail/go-crypto v1.1.3 // indirect github.com/Shopify/logrus-bugsnag v0.0.0-20171204204709-577dee27f20d // indirect github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d // indirect - github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751 // indirect github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 // indirect github.com/andrew-d/go-termutil v0.0.0-20150726205930-009166a695a2 // indirect github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect diff --git a/go.sum b/go.sum index 4fd8848a6..dc8b1d675 100644 --- a/go.sum +++ b/go.sum @@ -49,9 +49,9 @@ github.com/VictoriaMetrics/fastcache v1.12.0 h1:vnVi/y9yKDcD9akmc4NqAoqgQhJrOwUF github.com/VictoriaMetrics/fastcache v1.12.0/go.mod h1:tjiYeEfYXCqacuvYw/7UoDIeJaNxq6132xHICNP77w8= github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d h1:licZJFw2RwpHMqeKTCYkitsPqHNxTmd4SNR5r94FGM8= github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d/go.mod h1:asat636LX7Bqt5lYEZ27JNDcqxfjdBQuJ/MM4CN/Lzo= +github.com/alecthomas/kingpin/v2 v2.4.0 h1:f48lwail6p8zpO1bC4TxtqACaGqHYA22qkHjHpqDjYY= +github.com/alecthomas/kingpin/v2 v2.4.0/go.mod h1:0gyi0zQnjuFk8xrkNKamJoyUo382HRL7ATRpFZCw6tE= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= -github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751 h1:JYp7IbQjafoB+tBA3gMyHYHrpOtNuDiK/uB5uXxq5wM= -github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 h1:s6gZFSlWYmbqAuRjVTiNNhvNRfY2Wxp9nhfyel4rklc= github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137/go.mod h1:OMCwj8VM1Kc9e19TLln2VL61YJF0x1XFtfdL4JdbSyE= @@ -1015,7 +1015,6 @@ google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHh google.golang.org/protobuf v1.36.6 h1:z1NpPI8ku2WgiWnf+t9wTPsn6eP1L7ksHUlkfLvd9xY= google.golang.org/protobuf v1.36.6/go.mod h1:jduwjTPXsFjZGTmRluh+L6NjiWu7pchiJ2/5YcXBHnY= gopkg.in/airbrake/gobrake.v2 v2.0.9/go.mod h1:/h5ZAUhDkGaJfjzjKLSjv6zCL6O0LLBxU4K+aSYdM/U= -gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/cenkalti/backoff.v2 v2.2.1 h1:eJ9UAg01/HIHG987TwxvnzK2MgxXq97YY6rYDpY9aII= gopkg.in/cenkalti/backoff.v2 v2.2.1/go.mod h1:S0QdOvT2AlerfSBkp0O+dk+bbIMaNbEmVk876gPCthU=