Skip to content

add support for --tls flag (#203) #210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
feedmeapples merged 5 commits into from
Apr 29, 2023
Merged

add support for --tls flag (#203) #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ac053bc
Adding TLS flag
r-mol Apr 22, 2023
6a16f1c
Merge branch 'temporalio:main' into main
r-mol Apr 22, 2023
7c0d630
Update common/defs-flags.go
r-mol Apr 26, 2023
77daafe
Merge branch 'main' into main
feedmeapples Apr 28, 2023
b755b26
Update common/defs-flags.go
feedmeapples Apr 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions client/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,11 @@ func (b *clientFactory) createTLSConfig(c *cli.Context) (*tls.Config, error) {
if err != nil {
return nil, fmt.Errorf("unable to read TLS disable host verification flag: %w", err)
}
enableTLSS := c.String(common.FlagTLS)
enableTLS, err := strconv.ParseBool(enableTLSS)
Copy link
Contributor

@feedmeapples feedmeapples Apr 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the reason to do custom parsing? (instead of c.Bool(..))

Copy link
Contributor Author

@r-mol r-mol Apr 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm doing custom parsing to keep consistency with your code above:

cli/client/factory.go

Line 214 in 6a16f1c

disableHostNameVerificationS := c.String(common.FlagTLSDisableHostVerification)

Copy link
Contributor

@feedmeapples feedmeapples Apr 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok so these values are accepted with strconv.ParseBool, true: "1", "t", "T", "true", "TRUE", "True", false: "0", "f", "F", "false", "FALSE", "False"

I'd go with the c.Bool for consistency with the rest of the CLI

Copy link
Contributor

@feedmeapples feedmeapples Apr 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

if err != nil {
return nil, fmt.Errorf("unable to read TLS flag: %w", err)
}

serverName := c.String(common.FlagTLSServerName)

Expand Down Expand Up @@ -267,6 +272,17 @@ func (b *clientFactory) createTLSConfig(c *cli.Context) (*tls.Config, error) {
tlsConfig := auth.NewTLSConfigForServer(host, !disableHostNameVerification)
return tlsConfig, nil
}
// If we are given a TLS flag, set the TLS server name from the address
if enableTLS {
hostPort := c.String(common.FlagAddress)
if hostPort == "" {
hostPort = common.LocalHostPort
}
// Ignoring error as we'll fail to dial anyway, and that will produce a meaningful error
host, _, _ = net.SplitHostPort(hostPort)
tlsConfig := auth.NewTLSConfigForServer(host, !disableHostNameVerification)
return tlsConfig, nil
}

return nil, nil
}
Expand Down
1 change: 1 addition & 0 deletions common/defs-flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const (
FlagAddrDefinition = "The host and port (formatted as host:port) for the Temporal Frontend Service."
FlagNSAliasDefinition = "Identifies a Namespace in the Temporal Workflow."
FlagMetadataDefinition = "Contains gRPC metadata to send with requests (formatted as key=value)."
FlagTLSDefinition = "Enable TLS encryption without additional options such as mTLS or client certificates"
FlagTLSCertPathDefinition = "Path to x509 certificate."
FlagTLSKeyPathDefinition = "Path to private certificate key."
FlagTLSCaPathDefinition = "Path to server CA certificate."
Expand Down
7 changes: 7 additions & 0 deletions common/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ var (
FlagTaskQueueAlias = []string{"t"}
FlagTaskQueueType = "task-queue-type"
FlagTimeZone = "time-zone"
FlagTLS = "tls"
FlagTLSCaPath = "tls-ca-path"
FlagTLSCertPath = "tls-cert-path"
FlagTLSDisableHostVerification = "tls-disable-host-verification"
Expand Down Expand Up @@ -161,6 +162,12 @@ var SharedFlags = []cli.Flag{
Usage: FlagMetadataDefinition,
Category: CategoryGlobal,
},
&cli.BoolFlag{
Name: FlagTLS,
Usage: FlagTLSDefinition,
EnvVars: []string{"TEMPORAL_CLI_TLS"},
Category: CategoryGlobal,
},
&cli.StringFlag{
Name: FlagTLSCertPath,
Value: "",
Expand Down