Skip to content

Commit 64fa7fa

Browse files
authored
Tweak OAuth behavior (#913)
<!--- Note to EXTERNAL Contributors --> <!-- Thanks for opening a PR! If it is a significant code change, please **make sure there is an open issue** for this. We work best with you when we have accepted the idea first before you code. --> <!--- For ALL Contributors 👇 --> ## What was changed <!-- Describe what has changed in this PR --> (1) Ability to update the `address` env profile field when storing OAuth config. (2) Ability to use `${namespace}` in `address` to refer to namespace at runtime. (3) Show helpful error message to user in case they have OAuth but still use `default` namespace. ## Checklist <!--- add/delete as needed ---> 1. Closes <!-- add issue number here --> 2. How was this tested: <!--- Please describe how you tested your changes/how we can test them --> 3. Any docs updates needed? <!--- update README if applicable or point out where to update docs.temporal.io -->
1 parent 4670c5f commit 64fa7fa

4 files changed

Lines changed: 113 additions & 8 deletions

File tree

cliext/client.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ import (
1414
"google.golang.org/grpc"
1515
)
1616

17+
// addressNamespaceTemplate is the placeholder in addresses that gets replaced with the namespace.
18+
const addressNamespaceTemplate = "${namespace}"
19+
1720
// ClientOptionsBuilder contains options for building SDK client.Options.
1821
type ClientOptionsBuilder struct {
1922
// CommonOptions contains common CLI options including profile config.
@@ -103,12 +106,22 @@ func (b *ClientOptionsBuilder) Build(ctx context.Context) (client.Options, error
103106
} else if profile.Address == "" {
104107
profile.Address = cfg.Address
105108
}
109+
var namespaceExplicitlySet bool
106110
if cfg.FlagSet != nil && cfg.FlagSet.Changed("namespace") {
111+
namespaceExplicitlySet = true
107112
profile.Namespace = cfg.Namespace
108-
} else if profile.Namespace == "" {
113+
} else if profile.Namespace != "" {
114+
namespaceExplicitlySet = true
115+
} else {
109116
profile.Namespace = cfg.Namespace
110117
}
111118

119+
// Replace templated namespace in address, if present.
120+
addressHasNamespaceTemplate := strings.Contains(profile.Address, addressNamespaceTemplate)
121+
if addressHasNamespaceTemplate {
122+
profile.Address = strings.ReplaceAll(profile.Address, addressNamespaceTemplate, profile.Namespace)
123+
}
124+
112125
// Set API key on profile if provided
113126
if cfg.ApiKey != "" {
114127
profile.APIKey = cfg.ApiKey
@@ -214,6 +227,11 @@ func (b *ClientOptionsBuilder) Build(ctx context.Context) (client.Options, error
214227
}
215228
// Only set credentials if OAuth is configured with an access token
216229
if result.OAuth != nil && result.OAuth.Token != nil && result.OAuth.Token.AccessToken != "" {
230+
// Error if OAuth is configured with templated address but namespace was not explicitly set.
231+
if addressHasNamespaceTemplate && !namespaceExplicitlySet {
232+
return client.Options{}, fmt.Errorf(
233+
"namespace is required to be set via `--namespace` or configured in profile.")
234+
}
217235
creds := &oauthCredentials{
218236
builder: b,
219237
config: result.OAuth,

cliext/client_test.go

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,23 @@ func TestClientOptionsBuilder_OptionsPassedThrough(t *testing.T) {
8080
assert.NotNil(t, opts.HeadersProvider)
8181
}
8282

83+
func TestClientOptionsBuilder_NamespaceReplacementInAddress(t *testing.T) {
84+
builder := &cliext.ClientOptionsBuilder{
85+
CommonOptions: cliext.CommonOptions{
86+
DisableConfigFile: true,
87+
},
88+
ClientOptions: cliext.ClientOptions{
89+
Address: "${namespace}.api.temporal.io:7233",
90+
Namespace: "my-namespace",
91+
},
92+
}
93+
94+
opts, err := builder.Build(t.Context())
95+
96+
require.NoError(t, err)
97+
assert.Equal(t, "my-namespace.api.temporal.io:7233", opts.HostPort)
98+
}
99+
83100
func TestClientOptionsBuilder_OAuth_ValidToken(t *testing.T) {
84101
s := newMockOAuthServer(t)
85102
configFile := createTestOAuthConfig(t, s.tokenURL, time.Now().Add(time.Hour))
@@ -89,7 +106,8 @@ func TestClientOptionsBuilder_OAuth_ValidToken(t *testing.T) {
89106
ConfigFile: configFile,
90107
},
91108
ClientOptions: cliext.ClientOptions{
92-
Address: "localhost:17233",
109+
Address: "localhost:17233",
110+
Namespace: "my-namespace",
93111
},
94112
}
95113
opts, err := builder.Build(t.Context())
@@ -111,7 +129,8 @@ func TestClientOptionsBuilder_OAuth_Refresh(t *testing.T) {
111129
ConfigFile: configFile,
112130
},
113131
ClientOptions: cliext.ClientOptions{
114-
Address: "localhost:17233",
132+
Address: "localhost:17233",
133+
Namespace: "my-namespace",
115134
},
116135
}
117136
opts, err := builder.Build(context.Background())
@@ -141,7 +160,8 @@ func TestClientOptionsBuilder_OAuth_NoConfigFile(t *testing.T) {
141160
DisableConfigFile: true,
142161
},
143162
ClientOptions: cliext.ClientOptions{
144-
Address: "localhost:17233",
163+
Address: "localhost:17233",
164+
Namespace: "my-namespace",
145165
},
146166
}
147167
opts, err := builder.Build(t.Context())
@@ -160,7 +180,8 @@ func TestClientOptionsBuilder_OAuth_NoOAuthConfigured(t *testing.T) {
160180
ConfigFile: configFile,
161181
},
162182
ClientOptions: cliext.ClientOptions{
163-
Address: "localhost:17233",
183+
Address: "localhost:17233",
184+
Namespace: "my-namespace",
164185
},
165186
}
166187
opts, err := builder.Build(t.Context())
@@ -179,7 +200,8 @@ func TestClientOptionsBuilder_OAuth_DisableConfigFile(t *testing.T) {
179200
DisableConfigFile: true, // disables loading config file
180201
},
181202
ClientOptions: cliext.ClientOptions{
182-
Address: "localhost:17233",
203+
Address: "localhost:17233",
204+
Namespace: "my-namespace",
183205
},
184206
}
185207
opts, err := builder.Build(t.Context())
@@ -197,8 +219,9 @@ func TestClientOptionsBuilder_OAuth_APIKeyTakesPrecedence(t *testing.T) {
197219
ConfigFile: configFileWithOAuth,
198220
},
199221
ClientOptions: cliext.ClientOptions{
200-
Address: "localhost:17233",
201-
ApiKey: "explicit-api-key", // takes precedence
222+
Address: "localhost:17233",
223+
Namespace: "my-namespace",
224+
ApiKey: "explicit-api-key", // takes precedence
202225
},
203226
}
204227
opts, err := builder.Build(t.Context())
@@ -207,6 +230,25 @@ func TestClientOptionsBuilder_OAuth_APIKeyTakesPrecedence(t *testing.T) {
207230
assert.NotNil(t, opts.Credentials)
208231
}
209232

233+
func TestClientOptionsBuilder_OAuth_NonDefaultNamespaceRequired(t *testing.T) {
234+
configFileWithOAuth := createTestOAuthConfig(t, "", time.Now().Add(time.Hour))
235+
236+
builder := &cliext.ClientOptionsBuilder{
237+
CommonOptions: cliext.CommonOptions{
238+
ConfigFile: configFileWithOAuth,
239+
},
240+
ClientOptions: cliext.ClientOptions{
241+
Address: "${namespace}.api.temporal.io:7233",
242+
Namespace: "default",
243+
},
244+
}
245+
246+
_, err := builder.Build(t.Context())
247+
248+
require.Error(t, err)
249+
assert.Contains(t, err.Error(), "namespace is required")
250+
}
251+
210252
func createTestOAuthConfig(t *testing.T, tokenURL string, expiry time.Time) string {
211253
t.Helper()
212254
configFile := filepath.Join(t.TempDir(), "config.toml")

cliext/config.oauth.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ type StoreClientOAuthOptions struct {
169169
ProfileName string
170170
// OAuth is the OAuth configuration to store. If nil, removes OAuth from the profile.
171171
OAuth *OAuthConfig
172+
// Address is the server address to store in the profile. If empty, address is not modified.
173+
Address string
172174
// EnvLookup overrides environment variable lookup. If nil, uses os.LookupEnv.
173175
EnvLookup envconfig.EnvLookup
174176
}
@@ -215,6 +217,21 @@ func StoreClientOAuth(opts StoreClientOAuthOptions) error {
215217
return err
216218
}
217219

220+
// Set address if provided.
221+
if opts.Address != "" {
222+
profileSection, ok := existingRaw["profile"].(map[string]any)
223+
if !ok {
224+
profileSection = make(map[string]any)
225+
existingRaw["profile"] = profileSection
226+
}
227+
profile, ok := profileSection[profileName].(map[string]any)
228+
if !ok {
229+
profile = make(map[string]any)
230+
profileSection[profileName] = profile
231+
}
232+
profile["address"] = opts.Address
233+
}
234+
218235
// Marshal back to TOML.
219236
var buf bytes.Buffer
220237
enc := toml.NewEncoder(&buf)

cliext/config.oauth_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,3 +258,31 @@ address = "other:7233"
258258
assert.Contains(t, string(content), "other:7233")
259259
assert.Contains(t, string(content), "test-client")
260260
}
261+
262+
func TestStoreClientOAuth_WithAddress(t *testing.T) {
263+
f, err := os.CreateTemp("", "temporal-config-*.toml")
264+
require.NoError(t, err)
265+
defer os.Remove(f.Name())
266+
require.NoError(t, f.Close())
267+
268+
// Store OAuth config with address
269+
err = cliext.StoreClientOAuth(cliext.StoreClientOAuthOptions{
270+
ConfigFilePath: f.Name(),
271+
OAuth: &cliext.OAuthConfig{
272+
ClientConfig: &oauth2.Config{
273+
ClientID: "test-client",
274+
},
275+
Token: &oauth2.Token{
276+
AccessToken: "test-token",
277+
},
278+
},
279+
Address: "${namespace}.api.temporal.io:7233",
280+
})
281+
require.NoError(t, err)
282+
283+
// Verify address was written
284+
content, err := os.ReadFile(f.Name())
285+
require.NoError(t, err)
286+
assert.Contains(t, string(content), `address = "${namespace}.api.temporal.io:7233"`)
287+
assert.Contains(t, string(content), "test-client")
288+
}

0 commit comments

Comments
 (0)