Skip to content

Commit d5e663f

Browse files
committed
Support cli revoke command and allow config admin groups via env
Signed-off-by: Phan Duc <phan.duc@moneyforward.co.jp>
1 parent c2f51ef commit d5e663f

File tree

96 files changed

+233
-106
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

96 files changed

+233
-106
lines changed

CHANGELOG.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ items:
136136
title: Add ability for cluster admins to revoke other users' intercepts.
137137
body: >-
138138
The Traffic Manager now has a new API endpoint `RevokeIntercept` that can be used to revoke intercepts created by other users.
139-
This endpoint is only accessible to cluster admins and requires authentication via a kubernetestoken and membership in the `system:masters` or `telepresence:admin` group.
139+
This endpoint is only accessible to cluster admins and requires authentication via a kubernetestoken and membership in the `system:masters` or `telepresence:admin` or `kubeadm:cluster-admins` group.
140140
- version: 2.25.2
141141
date: 2025-12-26
142142
notes:

cmd/traffic/cmd/manager/managerutil/envconfig.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ type Env struct {
5656
AgentImagePullPolicy string `env:"AGENT_IMAGE_PULL_POLICY, parser=string, default="`
5757
AgentImagePullSecrets []core.LocalObjectReference `env:"AGENT_IMAGE_PULL_SECRETS, parser=json-local-refs,default="`
5858
AgentInjectPolicy agentconfig.InjectPolicy `env:"AGENT_INJECT_POLICY, parser=enable-policy, default=Never"`
59+
AgentK8sAdminGroups []string `env:"AGENT_K8S_ADMIN_GROUPS, parser=split-trim, default=system:masters"`
5960
AgentLogLevel string `env:"AGENT_LOG_LEVEL, parser=logLevel, defaultFrom=LogLevel"`
6061
AgentPort uint16 `env:"AGENT_PORT, parser=port-number, default=0"`
6162
AgentEnableH2cProbing bool `env:"AGENT_ENABLE_H2C_PROBING, parser=bool, default=false"`

cmd/traffic/cmd/manager/revoke_intercept_test.go

Lines changed: 84 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/datawire/dlib/dgroup"
1616
"github.com/datawire/dlib/dlog"
1717
rpc "github.com/telepresenceio/telepresence/rpc/v2/manager"
18+
"github.com/telepresenceio/telepresence/v2/cmd/traffic/cmd/manager/managerutil"
1819
"github.com/telepresenceio/telepresence/v2/cmd/traffic/cmd/manager/state"
1920
"github.com/telepresenceio/telepresence/v2/pkg/k8sapi"
2021
)
@@ -50,6 +51,16 @@ func TestRevokeIntercept_Authentication(t *testing.T) {
5051
wantErr: true,
5152
errMessage: "not found",
5253
},
54+
{
55+
name: "authorized user with kubeadm:cluster-admins",
56+
token: "valid-kubeadm-token",
57+
authenticated: true,
58+
username: "kubeadm-admin",
59+
groups: []string{"system:authenticated", "kubeadm:cluster-admins"},
60+
wantCode: codes.NotFound, // Will fail on intercept lookup, but auth passes
61+
wantErr: true,
62+
errMessage: "not found",
63+
},
5364
{
5465
name: "unauthorized user - not in allowed groups",
5566
token: "valid-user-token",
@@ -58,7 +69,7 @@ func TestRevokeIntercept_Authentication(t *testing.T) {
5869
groups: []string{"system:authenticated", "developers"},
5970
wantCode: codes.PermissionDenied,
6071
wantErr: true,
61-
errMessage: "user must be a member of telepresence:admin or system:masters group",
72+
errMessage: "user must be a member of one of the following groups",
6273
},
6374
{
6475
name: "unauthenticated token",
@@ -81,11 +92,11 @@ func TestRevokeIntercept_Authentication(t *testing.T) {
8192
errMessage: "authentication failed",
8293
},
8394
{
84-
name: "user with both groups",
95+
name: "user with multiple admin groups",
8596
token: "super-admin-token",
8697
authenticated: true,
8798
username: "super-admin",
88-
groups: []string{"system:authenticated", "system:masters", "telepresence:admin"},
99+
groups: []string{"system:authenticated", "system:masters", "telepresence:admin", "kubeadm:cluster-admins"},
89100
wantCode: codes.NotFound, // Will fail on intercept lookup, but auth passes
90101
wantErr: true,
91102
errMessage: "not found",
@@ -123,6 +134,12 @@ func TestRevokeIntercept_Authentication(t *testing.T) {
123134
// Set up context with fake K8s client
124135
ctx = k8sapi.WithK8sInterface(ctx, fakeClient)
125136

137+
// Set up environment with default admin groups
138+
env := &managerutil.Env{
139+
AgentK8sAdminGroups: []string{"system:masters", "telepresence:admin", "kubeadm:cluster-admins"},
140+
}
141+
ctx = managerutil.WithEnv(ctx, env)
142+
126143
// Create a minimal service instance
127144
g := dgroup.NewGroup(ctx, dgroup.GroupConfig{})
128145
svc := &service{
@@ -176,6 +193,12 @@ func TestRevokeIntercept_SuccessfulRevocation(t *testing.T) {
176193
// Set up context with fake K8s client
177194
ctx = k8sapi.WithK8sInterface(ctx, fakeClient)
178195

196+
// Set up environment with default admin groups
197+
env := &managerutil.Env{
198+
AgentK8sAdminGroups: []string{"system:masters"},
199+
}
200+
ctx = managerutil.WithEnv(ctx, env)
201+
179202
// Create a service instance with state
180203
g := dgroup.NewGroup(ctx, dgroup.GroupConfig{})
181204
svc := &service{
@@ -229,7 +252,7 @@ func TestRevokeIntercept_SuccessfulRevocation(t *testing.T) {
229252
}
230253

231254
func TestRevokeIntercept_OnlySystemMasters(t *testing.T) {
232-
// This test specifically verifies that ONLY system:masters (or telepresence:admin) can revoke
255+
// This test specifically verifies that ONLY configured admin groups can revoke
233256
unauthorizedGroups := [][]string{
234257
{"system:authenticated"},
235258
{"system:authenticated", "developers"},
@@ -261,6 +284,12 @@ func TestRevokeIntercept_OnlySystemMasters(t *testing.T) {
261284

262285
ctx = k8sapi.WithK8sInterface(ctx, fakeClient)
263286

287+
// Set up environment with default admin groups
288+
env := &managerutil.Env{
289+
AgentK8sAdminGroups: []string{"system:masters", "telepresence:admin", "kubeadm:cluster-admins"},
290+
}
291+
ctx = managerutil.WithEnv(ctx, env)
292+
264293
g := dgroup.NewGroup(ctx, dgroup.GroupConfig{})
265294
svc := &service{
266295
state: state.NewState(ctx, g),
@@ -277,9 +306,59 @@ func TestRevokeIntercept_OnlySystemMasters(t *testing.T) {
277306
st, ok := status.FromError(err)
278307
require.True(t, ok)
279308
assert.Equal(t, codes.PermissionDenied, st.Code())
280-
assert.Contains(t, st.Message(), "user must be a member of telepresence:admin or system:masters group")
309+
assert.Contains(t, st.Message(), "user must be a member of one of the following groups")
281310

282311
t.Logf("Correctly denied access for groups: %v", groups)
283312
})
284313
}
285314
}
315+
316+
func TestRevokeIntercept_CustomAdminGroups(t *testing.T) {
317+
// Test with custom admin groups from environment
318+
ctx := dlog.NewTestContext(t, true)
319+
320+
fakeClient := fake.NewClientset()
321+
fakeClient.PrependReactor("create", "tokenreviews", func(action k8stesting.Action) (bool, runtime.Object, error) {
322+
createAction := action.(k8stesting.CreateAction)
323+
tr := createAction.GetObject().(*authv1.TokenReview)
324+
325+
// Return authenticated user with test-admin service account
326+
tr.Status = authv1.TokenReviewStatus{
327+
Authenticated: true,
328+
User: authv1.UserInfo{
329+
Username: "system:serviceaccount:ambassador:test-admin",
330+
Groups: []string{"system:serviceaccounts", "system:serviceaccounts:ambassador", "system:authenticated"},
331+
},
332+
}
333+
334+
return true, tr, nil
335+
})
336+
337+
ctx = k8sapi.WithK8sInterface(ctx, fakeClient)
338+
339+
// Set up environment with custom admin groups including the service account
340+
env := &managerutil.Env{
341+
AgentK8sAdminGroups: []string{"system:masters", "system:serviceaccount:ambassador:test-admin"},
342+
}
343+
ctx = managerutil.WithEnv(ctx, env)
344+
345+
g := dgroup.NewGroup(ctx, dgroup.GroupConfig{})
346+
svc := &service{
347+
state: state.NewState(ctx, g),
348+
}
349+
350+
req := &rpc.RevokeInterceptRequest{
351+
InterceptId: "test:intercept",
352+
Token: "test-token",
353+
}
354+
355+
_, err := svc.RevokeIntercept(ctx, req)
356+
357+
// Should get NotFound (auth passed, but intercept doesn't exist) instead of PermissionDenied
358+
require.Error(t, err)
359+
st, ok := status.FromError(err)
360+
require.True(t, ok)
361+
// Auth should pass, so we get NotFound instead of PermissionDenied
362+
assert.Equal(t, codes.NotFound, st.Code(), "Expected NotFound since auth passed but intercept doesn't exist")
363+
t.Logf("Successfully verified that custom admin group (system:serviceaccount:ambassador:test-admin) is allowed")
364+
}

cmd/traffic/cmd/manager/service.go

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -885,19 +885,33 @@ func (s *service) RemoveIntercept(ctx context.Context, riReq *rpc.RemoveIntercep
885885

886886
// RevokeIntercept allows the manager to revoke any client's intercept by intercept ID.
887887
// This is an administrative operation that can revoke intercepts for any client.
888-
// Requires authentication via token and membership in system:telepresence-admin group.
888+
// Requires authentication via token and membership in one of the groups specified
889+
// by the AGENT_K8S_ADMIN_GROUPS environment variable (default: system:masters).
889890
func (s *service) RevokeIntercept(ctx context.Context, riReq *rpc.RevokeInterceptRequest) (*empty.Empty, error) {
890891
// Verify the authentication token
891-
tokenResult, error := k8sapi.VerifyToken(ctx, riReq.Token)
892-
if error != nil {
893-
dlog.Warnf(ctx, "Authentication failed for RevokeIntercept: %v", error)
892+
tokenResult, err := k8sapi.VerifyToken(ctx, riReq.Token)
893+
if err != nil {
894+
dlog.Warnf(ctx, "Authentication failed for RevokeIntercept: %v", err)
894895
return nil, status.Errorf(codes.PermissionDenied, "authentication failed")
895896
}
896897

897-
// Check if user belongs to system:telepresence-admin group
898-
if !(tokenResult.IsMemberOfGroup("telepresence:admin") || tokenResult.IsMemberOfGroup("system:masters")) {
899-
dlog.Warnf(ctx, "User %s is not a member of telepresence:admin or system:masters group", tokenResult.Username)
900-
return nil, status.Errorf(codes.PermissionDenied, "user must be a member of telepresence:admin or system:masters group")
898+
// Check if user belongs to one of the allowed admin groups
899+
allowedGroups := managerutil.GetEnv(ctx).AgentK8sAdminGroups
900+
if len(allowedGroups) == 0 {
901+
// Default to system:masters if not configured
902+
allowedGroups = []string{"system:masters"}
903+
}
904+
isAuthorized := false
905+
for _, group := range allowedGroups {
906+
if tokenResult.IsMemberOfGroup(group) {
907+
isAuthorized = true
908+
break
909+
}
910+
}
911+
if !isAuthorized {
912+
groupsStr := strings.Join(allowedGroups, ", ")
913+
dlog.Warnf(ctx, "User %s is not a member of any allowed admin groups: %s", tokenResult.Username, groupsStr)
914+
return nil, status.Errorf(codes.PermissionDenied, "user must be a member of one of the following groups: %s", groupsStr)
901915
}
902916

903917
dlog.Infof(ctx, "Authorized to revoke intercepts")

docs/reference/cli/telepresence.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ recommended) which in turn may result in a password prompt.
5656
| [mcp](telepresence_mcp) | MCP server management |
5757
| [quit](telepresence_quit) | Tell telepresence daemons to quit |
5858
| [replace](telepresence_replace) | Replace a container |
59+
| [revoke](telepresence_revoke) | Revoke an intercept by intercept ID. The intercept ID must be in the format <session_id>:<intercept_name> |
5960
| [serve](telepresence_serve) | Start the browser on a remote service |
6061
| [status](telepresence_status) | Show connectivity status |
6162
| [uninstall](telepresence_uninstall) | Uninstall telepresence agents |
@@ -69,7 +70,7 @@ recommended) which in turn may result in a password prompt.
6970

7071
### Global Flags:
7172
```
72-
--config string Path to the Telepresence configuration file (default "$HOME/Library/Application Support/telepresence/config.yml")
73+
--config string Path to the Telepresence configuration file (default "$HOME/.config/telepresence/config.yml")
7374
--output string Set the output format, supported values are 'json', 'yaml', and 'default' (default "default")
7475
--progress string Set type of progress output (auto, tty, plain, json, quiet) (default "auto")
7576
--use string Match expression that uniquely identifies the daemon container

docs/reference/cli/telepresence_completion.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ To load completions:
6666

6767
### Global Flags:
6868
```
69-
--config string Path to the Telepresence configuration file (default "$HOME/Library/Application Support/telepresence/config.yml")
69+
--config string Path to the Telepresence configuration file (default "$HOME/.config/telepresence/config.yml")
7070
--output string Set the output format, supported values are 'json', 'yaml', and 'default' (default "default")
7171
--progress string Set type of progress output (auto, tty, plain, json, quiet) (default "auto")
7272
--use string Match expression that uniquely identifies the daemon container

docs/reference/cli/telepresence_compose.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ Define and run multi-container applications with Telepresence and Docker
6666

6767
### Global Flags:
6868
```
69-
--config string Path to the Telepresence configuration file (default "$HOME/Library/Application Support/telepresence/config.yml")
69+
--config string Path to the Telepresence configuration file (default "$HOME/.config/telepresence/config.yml")
7070
--output string Set the output format, supported values are 'json', 'yaml', and 'default' (default "default")
7171
--progress string Set type of progress output (auto, tty, plain, json, quiet) (default "auto")
7272
--use string Match expression that uniquely identifies the daemon container

docs/reference/cli/telepresence_compose_attach.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ Attach local standard input, output, and error streams to a service's running co
3535

3636
### Global Flags:
3737
```
38-
--config string Path to the Telepresence configuration file (default "$HOME/Library/Application Support/telepresence/config.yml")
38+
--config string Path to the Telepresence configuration file (default "$HOME/.config/telepresence/config.yml")
3939
--output string Set the output format, supported values are 'json', 'yaml', and 'default' (default "default")
4040
--progress string Set type of progress output (auto, tty, plain, json, quiet) (default "auto")
4141
--use string Match expression that uniquely identifies the daemon container

docs/reference/cli/telepresence_compose_bridge.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ Convert compose files into another model
3232

3333
### Global Flags:
3434
```
35-
--config string Path to the Telepresence configuration file (default "$HOME/Library/Application Support/telepresence/config.yml")
35+
--config string Path to the Telepresence configuration file (default "$HOME/.config/telepresence/config.yml")
3636
--output string Set the output format, supported values are 'json', 'yaml', and 'default' (default "default")
3737
--progress string Set type of progress output (auto, tty, plain, json, quiet) (default "auto")
3838
--use string Match expression that uniquely identifies the daemon container

docs/reference/cli/telepresence_compose_build.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ Build or rebuild services
4444

4545
### Global Flags:
4646
```
47-
--config string Path to the Telepresence configuration file (default "$HOME/Library/Application Support/telepresence/config.yml")
47+
--config string Path to the Telepresence configuration file (default "$HOME/.config/telepresence/config.yml")
4848
--output string Set the output format, supported values are 'json', 'yaml', and 'default' (default "default")
4949
--progress string Set type of progress output (auto, tty, plain, json, quiet) (default "auto")
5050
--use string Match expression that uniquely identifies the daemon container

0 commit comments

Comments
 (0)