polish

Author: stephanos

.github/workflows/ci.yaml

@@ -60,7 +60,7 @@ jobs:
       - name: Regen code, confirm unchanged
         if: ${{ matrix.checkGenCommands }}
         run: |
-          go run ./cmd/gen-commands -input internal/temporalcli/commands.yaml -input cliext/option-sets.yaml -pkg temporalcli -context "*CommandContext" > internal/temporalcli/commands.gen.go
+          go run ./cmd/gen-commands -input internal/temporalcli/commands.yaml -pkg temporalcli -context "*CommandContext" > internal/temporalcli/commands.gen.go
           go run ./cmd/gen-commands -input cliext/option-sets.yaml -pkg cliext > cliext/flags.gen.go
           git diff --exit-code
 

CONTRIBUTING.md

@@ -31,7 +31,7 @@ This will expect every non-parent command to have a `run` method, so for new com
 Once a command is updated, the CI will automatically generate new docs
 and create a PR in the Documentation repo with the corresponding updates. To generate these docs locally, run:
 
-    go run ./cmd/gen-docs -input internal/temporalcli/commands.yaml -output dist/docs
+    go run ./cmd/gen-docs -input internal/temporalcli/commands.yaml -input cliext/option-sets.yaml -output dist/docs
 
 This will auto-generate a new set of docs to `dist/docs/`. If a new root command is added, a new file will be automatically generated, like `temporal activity` and `activity.mdx`.
 

Makefile

@@ -4,15 +4,12 @@ all: gen build
 
 gen: internal/temporalcli/commands.gen.go cliext/flags.gen.go
 
-# Generate CLI commands code from commands.yaml, using option sets from cliext
-internal/temporalcli/commands.gen.go: internal/temporalcli/commands.yaml cliext/option-sets.yaml
+internal/temporalcli/commands.gen.go: internal/temporalcli/commands.yaml
 	go run ./cmd/gen-commands \
 		-input internal/temporalcli/commands.yaml \
-		-input cliext/option-sets.yaml \
 		-pkg temporalcli \
 		-context "*CommandContext" > $@
 
-# Generate cliext flags code from option-sets.yaml (no commands = cliext mode)
 cliext/flags.gen.go: cliext/option-sets.yaml
 	go run ./cmd/gen-commands \
 		-input cliext/option-sets.yaml \

cliext/client.go

@@ -2,18 +2,16 @@ package cliext
 
 import (
 	"context"
-	"crypto/tls"
-	"crypto/x509"
 	"fmt"
 	"log/slog"
 	"net/http"
-	"os"
 	"strings"
 
 	"go.temporal.io/sdk/client"
 	"go.temporal.io/sdk/contrib/envconfig"
 	"go.temporal.io/sdk/converter"
 	"go.temporal.io/sdk/log"
+	"golang.org/x/oauth2"
 	"google.golang.org/grpc"
 )
 
@@ -29,14 +27,14 @@ type ClientOptionsBuilder struct {
 	// Logger is the slog logger to use for the client. If set, it will be
 	// wrapped with the SDK's structured logger adapter.
 	Logger *slog.Logger
+	// oauthTokenSource is initialized during Build() if OAuth is configured.
+	oauthTokenSource oauth2.TokenSource
 }
 
-// BuildClientOptions creates SDK client options from a ClientOptionsBuilder.
-// If OAuth is configured and no APIKey is set, OAuth will be used to obtain an access token.
-// Returns the client options and the resolved namespace (which may differ from input if loaded from profile).
-func BuildClientOptions(ctx context.Context, opts ClientOptionsBuilder) (client.Options, string, error) {
-	cfg := opts.ClientOptions
-	common := opts.CommonOptions
+// Build creates SDK client.Options from the ClientOptionsBuilder options.
+func (b *ClientOptionsBuilder) Build(ctx context.Context) (client.Options, error) {
+	cfg := b.ClientOptions
+	common := b.CommonOptions
 
 	// Load a client config profile if configured
 	var profile envconfig.ClientConfigProfile
@@ -47,22 +45,22 @@ func BuildClientOptions(ctx context.Context, opts ClientOptionsBuilder) (client.
 			ConfigFileProfile: common.Profile,
 			DisableFile:       common.DisableConfigFile,
 			DisableEnv:        common.DisableConfigEnv,
-			EnvLookup:         opts.EnvLookup,
+			EnvLookup:         b.EnvLookup,
 		})
 		if err != nil {
-			return client.Options{}, "", fmt.Errorf("failed loading client config: %w", err)
+			return client.Options{}, fmt.Errorf("failed loading client config: %w", err)
 		}
 	}
 
 	// To support legacy TLS environment variables, if they are present, we will
 	// have them force-override anything loaded from existing file or env
-	if !common.DisableConfigEnv && opts.EnvLookup != nil {
-		oldEnvTLSCert, _ := opts.EnvLookup.LookupEnv("TEMPORAL_TLS_CERT")
-		oldEnvTLSCertData, _ := opts.EnvLookup.LookupEnv("TEMPORAL_TLS_CERT_DATA")
-		oldEnvTLSKey, _ := opts.EnvLookup.LookupEnv("TEMPORAL_TLS_KEY")
-		oldEnvTLSKeyData, _ := opts.EnvLookup.LookupEnv("TEMPORAL_TLS_KEY_DATA")
-		oldEnvTLSCA, _ := opts.EnvLookup.LookupEnv("TEMPORAL_TLS_CA")
-		oldEnvTLSCAData, _ := opts.EnvLookup.LookupEnv("TEMPORAL_TLS_CA_DATA")
+	if !common.DisableConfigEnv && b.EnvLookup != nil {
+		oldEnvTLSCert, _ := b.EnvLookup.LookupEnv("TEMPORAL_TLS_CERT")
+		oldEnvTLSCertData, _ := b.EnvLookup.LookupEnv("TEMPORAL_TLS_CERT_DATA")
+		oldEnvTLSKey, _ := b.EnvLookup.LookupEnv("TEMPORAL_TLS_KEY")
+		oldEnvTLSKeyData, _ := b.EnvLookup.LookupEnv("TEMPORAL_TLS_KEY_DATA")
+		oldEnvTLSCA, _ := b.EnvLookup.LookupEnv("TEMPORAL_TLS_CA")
+		oldEnvTLSCAData, _ := b.EnvLookup.LookupEnv("TEMPORAL_TLS_CA_DATA")
 		if oldEnvTLSCert != "" || oldEnvTLSCertData != "" ||
 			oldEnvTLSKey != "" || oldEnvTLSKeyData != "" ||
 			oldEnvTLSCA != "" || oldEnvTLSCAData != "" {
@@ -96,13 +94,10 @@ func BuildClientOptions(ctx context.Context, opts ClientOptionsBuilder) (client.
 	if cfg.FlagSet != nil && cfg.FlagSet.Changed("address") {
 		profile.Address = cfg.Address
 	}
-	resolvedNamespace := profile.Namespace
 	if cfg.FlagSet != nil && cfg.FlagSet.Changed("namespace") {
 		profile.Namespace = cfg.Namespace
-		resolvedNamespace = cfg.Namespace
 	} else if profile.Namespace == "" {
 		profile.Namespace = cfg.Namespace
-		resolvedNamespace = cfg.Namespace
 	}
 
 	// Set API key on profile if provided (OAuth credentials are set later on clientOpts)
@@ -114,7 +109,7 @@ func BuildClientOptions(ctx context.Context, opts ClientOptionsBuilder) (client.
 	if len(cfg.GrpcMeta) > 0 {
 		grpcMetaFromArg, err := parseKeyValuePairs(cfg.GrpcMeta)
 		if err != nil {
-			return client.Options{}, "", fmt.Errorf("invalid gRPC meta: %w", err)
+			return client.Options{}, fmt.Errorf("invalid gRPC meta: %w", err)
 		}
 		if len(profile.GRPCMeta) == 0 {
 			profile.GRPCMeta = make(map[string]string, len(cfg.GrpcMeta))
@@ -125,6 +120,8 @@ func BuildClientOptions(ctx context.Context, opts ClientOptionsBuilder) (client.
 	}
 
 	// If any of these TLS values are present, set TLS if not set, and set values.
+	// NOTE: This means that tls=false does not explicitly disable TLS when set
+	// via envconfig.
 	if cfg.Tls ||
 		cfg.TlsCertPath != "" || cfg.TlsKeyPath != "" || cfg.TlsCaPath != "" ||
 		cfg.TlsCertData != "" || cfg.TlsKeyData != "" || cfg.TlsCaData != "" {
@@ -178,7 +175,12 @@ func BuildClientOptions(ctx context.Context, opts ClientOptionsBuilder) (client.
 	// Convert profile to client options.
 	clientOpts, err := profile.ToClientOptions(envconfig.ToClientOptionsRequest{})
 	if err != nil {
-		return client.Options{}, "", fmt.Errorf("failed to build client options: %w", err)
+		return client.Options{}, fmt.Errorf("failed to build client options: %w", err)
+	}
+
+	// Set client authority if provided.
+	if cfg.ClientAuthority != "" {
+		clientOpts.ConnectionOptions.Authority = cfg.ClientAuthority
 	}
 
 	// Set identity if provided.
@@ -187,43 +189,42 @@ func BuildClientOptions(ctx context.Context, opts ClientOptionsBuilder) (client.
 	}
 
 	// Set logger if provided.
-	if opts.Logger != nil {
-		clientOpts.Logger = log.NewStructuredLogger(opts.Logger)
+	if b.Logger != nil {
+		clientOpts.Logger = log.NewStructuredLogger(b.Logger)
 	}
 
-	// Set OAuth credentials if configured and no API key is set.
-	// OAuth config is loaded on-demand from the config file.
+	// Initialize OAuth token source if no API key is set.
 	if cfg.ApiKey == "" {
-		clientOpts.Credentials = client.NewAPIKeyDynamicCredentials(
-			NewOAuthDynamicTokenProvider(opts))
-	}
-
-	// Set client authority if provided.
-	if cfg.ClientAuthority != "" {
-		clientOpts.ConnectionOptions.Authority = cfg.ClientAuthority
-	}
-
-	// Set connect timeout for GetSystemInfo if provided.
-	if common.ClientConnectTimeout != 0 {
-		clientOpts.ConnectionOptions.GetSystemInfoTimeout = common.ClientConnectTimeout.Duration()
+		if err := b.initOAuthTokenSource(ctx); err != nil {
+			return client.Options{}, fmt.Errorf("failed to initialize OAuth: %w", err)
+		}
+		// Only set credentials if OAuth is configured
+		if b.oauthTokenSource != nil {
+			clientOpts.Credentials = client.NewAPIKeyDynamicCredentials(b.getOAuthToken)
+		}
 	}
 
-	// Add codec interceptor if codec endpoint is configured.
+	// Remote codec
 	if profile.Codec != nil && profile.Codec.Endpoint != "" {
 		codecHeaders, err := parseKeyValuePairs(cfg.CodecHeader)
 		if err != nil {
-			return client.Options{}, "", fmt.Errorf("invalid codec headers: %w", err)
+			return client.Options{}, fmt.Errorf("invalid codec headers: %w", err)
 		}
 		interceptor, err := newPayloadCodecInterceptor(
 			profile.Namespace, profile.Codec.Endpoint, profile.Codec.Auth, codecHeaders)
 		if err != nil {
-			return client.Options{}, "", fmt.Errorf("failed creating payload codec interceptor: %w", err)
+			return client.Options{}, fmt.Errorf("failed creating payload codec interceptor: %w", err)
 		}
 		clientOpts.ConnectionOptions.DialOptions = append(
 			clientOpts.ConnectionOptions.DialOptions, grpc.WithChainUnaryInterceptor(interceptor))
 	}
 
-	return clientOpts, resolvedNamespace, nil
+	// Set connect timeout for GetSystemInfo if provided.
+	if common.ClientConnectTimeout != 0 {
+		clientOpts.ConnectionOptions.GetSystemInfoTimeout = common.ClientConnectTimeout.Duration()
+	}
+
+	return clientOpts, nil
 }
 
 // parseKeyValuePairs parses a slice of "KEY=VALUE" strings into a map.
@@ -270,52 +271,36 @@ func newPayloadCodecInterceptor(
 	)
 }
 
-// BuildTLSConfig creates a TLS configuration from the ClientOptions TLS settings.
-// This is useful when you need the TLS config separately from the full client options.
-func BuildTLSConfig(cfg ClientOptions) (*tls.Config, error) {
-	if !cfg.Tls && cfg.TlsCertPath == "" && cfg.TlsKeyPath == "" && cfg.TlsCaPath == "" &&
-		cfg.TlsCertData == "" && cfg.TlsKeyData == "" && cfg.TlsCaData == "" {
-		return nil, nil
+// getOAuthToken returns a valid OAuth access token from the builder's configuration.
+// It uses the oauth2.TokenSource to automatically refresh the token when needed.
+// Returns empty string if no OAuth is configured.
+func (b *ClientOptionsBuilder) getOAuthToken(ctx context.Context) (string, error) {
+	if b.oauthTokenSource == nil {
+		return "", nil
 	}
-
-	tlsConfig := &tls.Config{
-		ServerName:         cfg.TlsServerName,
-		InsecureSkipVerify: cfg.TlsDisableHostVerification,
+	token, err := b.oauthTokenSource.Token()
+	if err != nil {
+		return "", err
 	}
+	return token.AccessToken, nil
+}
 
-	// Load client certificate.
-	if cfg.TlsCertPath != "" && cfg.TlsKeyPath != "" {
-		cert, err := tls.LoadX509KeyPair(cfg.TlsCertPath, cfg.TlsKeyPath)
-		if err != nil {
-			return nil, fmt.Errorf("failed to load client certificate: %w", err)
-		}
-		tlsConfig.Certificates = []tls.Certificate{cert}
-	} else if cfg.TlsCertData != "" && cfg.TlsKeyData != "" {
-		cert, err := tls.X509KeyPair([]byte(cfg.TlsCertData), []byte(cfg.TlsKeyData))
-		if err != nil {
-			return nil, fmt.Errorf("failed to parse client certificate: %w", err)
-		}
-		tlsConfig.Certificates = []tls.Certificate{cert}
+// initOAuthTokenSource initializes the OAuth token source from the configuration.
+// This should be called once during Build() to load the OAuth config and create
+// the token source. The token source is then bound to the builder and used for
+// all subsequent token requests.
+func (b *ClientOptionsBuilder) initOAuthTokenSource(ctx context.Context) error {
+	result, err := LoadClientOAuth(LoadClientOAuthOptions{
+		ConfigFilePath: b.CommonOptions.ConfigFile,
+		ProfileName:    b.CommonOptions.Profile,
+		EnvLookup:      b.EnvLookup,
+	})
+	if err != nil {
+		return err
 	}
 
-	// Load CA certificate.
-	if cfg.TlsCaPath != "" || cfg.TlsCaData != "" {
-		pool := x509.NewCertPool()
-		var caData []byte
-		if cfg.TlsCaPath != "" {
-			var err error
-			caData, err = os.ReadFile(cfg.TlsCaPath)
-			if err != nil {
-				return nil, fmt.Errorf("failed to read CA certificate: %w", err)
-			}
-		} else {
-			caData = []byte(cfg.TlsCaData)
-		}
-		if !pool.AppendCertsFromPEM(caData) {
-			return nil, fmt.Errorf("failed to parse CA certificate")
-		}
-		tlsConfig.RootCAs = pool
+	if result.OAuth != nil && result.OAuth.RefreshToken != "" {
+		b.oauthTokenSource = result.OAuth.newTokenSource(ctx)
 	}
-
-	return tlsConfig, nil
+	return nil
 }