Skip to content

Commit

Permalink
fix: Default to insecure mode when no certs are present (#5511)
Browse files Browse the repository at this point in the history
Signed-off-by: Simon Behar <simbeh7@gmail.com>
  • Loading branch information
simster7 authored Mar 26, 2021
1 parent 4a1caca commit a61d84c
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 8 deletions.
30 changes: 22 additions & 8 deletions cmd/argo/commands/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

eventsource "github.com/argoproj/argo-events/pkg/client/eventsource/clientset/versioned"
sensor "github.com/argoproj/argo-events/pkg/client/sensor/clientset/versioned"
"github.com/argoproj/pkg/errors"
"github.com/argoproj/pkg/stats"
log "github.com/sirupsen/logrus"
"github.com/skratchdot/open-golang/open"
Expand Down Expand Up @@ -51,13 +50,15 @@ func NewServerCommand() *cobra.Command {
Short: "start the Argo Server",
Example: fmt.Sprintf(`
See %s`, help.ArgoSever),
Run: func(c *cobra.Command, args []string) {
RunE: func(c *cobra.Command, args []string) error {
cmd.SetLogFormatter(logFormat)
stats.RegisterStackDumper()
stats.StartStatsTicker(5 * time.Minute)

config, err := client.GetConfig().ClientConfig()
errors.CheckError(err)
if err != nil {
return err
}
config.Burst = 30
config.QPS = 20.0

Expand Down Expand Up @@ -90,9 +91,13 @@ See %s`, help.ArgoSever),
var tlsConfig *tls.Config
if secure {
cer, err := tls.LoadX509KeyPair("argo-server.crt", "argo-server.key")
errors.CheckError(err)
if err != nil {
return err
}
tlsMinVersion, err := env.GetInt("TLS_MIN_VERSION", tls.VersionTLS12)
errors.CheckError(err)
if err != nil {
return err
}
tlsConfig = &tls.Config{
Certificates: []tls.Certificate{cer},
InsecureSkipVerify: false, // InsecureSkipVerify will not impact the TLS listener. It is needed for the server to speak to itself for GRPC.
Expand All @@ -105,7 +110,9 @@ See %s`, help.ArgoSever),
modes := auth.Modes{}
for _, mode := range authModes {
err := modes.Add(mode)
errors.CheckError(err)
if err != nil {
return err
}
}
if reflect.DeepEqual(modes, auth.Modes{auth.Server: true}) {
log.Warn("You are running without client authentication. Learn how to enable client authentication: https://argoproj.github.io/argo-workflows/argo-server-auth-mode/")
Expand Down Expand Up @@ -136,9 +143,14 @@ See %s`, help.ArgoSever),
}
}
}

server, err := apiserver.NewArgoServer(ctx, opts)
errors.CheckError(err)
if err != nil {
return err
}

server.Run(ctx, port, browserOpenFunc)
return nil
},
}

Expand All @@ -149,7 +161,9 @@ See %s`, help.ArgoSever),
}
command.Flags().StringVar(&baseHRef, "basehref", defaultBaseHRef, "Value for base href in index.html. Used if the server is running behind reverse proxy under subpath different from /. Defaults to the environment variable BASE_HREF.")
// "-e" for encrypt, like zip
command.Flags().BoolVarP(&secure, "secure", "e", true, "Whether or not we should listen on TLS.")
// We default to secure mode if we find certs available, otherwise we default to insecure mode.
_, err := os.Stat("argo-server.crt")
command.Flags().BoolVarP(&secure, "secure", "e", !os.IsNotExist(err), "Whether or not we should listen on TLS.")
command.Flags().BoolVar(&htst, "hsts", true, "Whether or not we should add a HTTP Secure Transport Security header. This only has effect if secure is enabled.")
command.Flags().StringArrayVar(&authModes, "auth-mode", []string{"client"}, "API server authentication mode. Any 1 or more length permutation of: client,server,sso")
command.Flags().StringVar(&configMap, "configmap", "workflow-controller-configmap", "Name of K8s configmap to retrieve workflow controller configuration")
Expand Down
26 changes: 26 additions & 0 deletions cmd/argo/commands/server_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package commands

import (
"github.com/stretchr/testify/assert"
"os"
"testing"
)

func TestDefaultSecureMode(t *testing.T) {
// No certs: We should run insecure
cmd := NewServerCommand()
assert.Equal(t, "false", cmd.Flag("secure").Value.String())

// Clean up and delete tests files
defer func() {
_ = os.Remove("argo-server.crt")
_ = os.Remove("argo-server.key")
}()

_, _ = os.Create("argo-server.crt")
_, _ = os.Create("argo-server.key")

// No certs: We should secure
cmd = NewServerCommand()
assert.Equal(t, "true", cmd.Flag("secure").Value.String())
}

0 comments on commit a61d84c

Please sign in to comment.