Skip to content

Support additional expected instance roles. #25718

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
mdwn merged 3 commits into from
May 5, 2023
Merged

Support additional expected instance roles. #25718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ff19b8
Support additional expected instance roles.
May 5, 2023
99724c2
Correct comments, add in a test.
May 5, 2023
dec3575
Add in godoc.
May 5, 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
6 changes: 6 additions & 0 deletions lib/service/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4685,6 +4685,12 @@ func (process *TeleportProcess) waitForAppDepend() {

// registerExpectedServices sets up the instance role -> identity event mapping.
func (process *TeleportProcess) registerExpectedServices(cfg *servicecfg.Config) {
// Register additional expected services for this Teleport instance.
// Meant for enterprise support.
for _, r := range cfg.AdditionalExpectedRoles {
Copy link
Copy Markdown
Collaborator

@r0mant r0mant May 5, 2023

Choose a reason for hiding this comment

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

Also, why do we need to pass this to OSS via AdditionalExpectedRoles, can't enterprise code just use SetExpectedInstanceRole(...) on the process when initializing appropriate service?

Copy link
Copy Markdown
Contributor Author

@mdwn mdwn May 5, 2023

Choose a reason for hiding this comment

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

It cannot, unfortunately. Basically the inventory status message is created at the very beginning of NewTeleport, so the enterprise statuses never actually get the roles from SetExpectedInstanceRole.

process.SetExpectedInstanceRole(r.Role, r.IdentityEvent)
}
Comment on lines +4688 to +4692
Copy link
Copy Markdown
Collaborator

@r0mant r0mant May 5, 2023

Choose a reason for hiding this comment

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

Super dumb question but if these are "additional roles", then why is this block right at the front of the function rather than after all "regular expected roles" below? :)

Copy link
Copy Markdown
Contributor Author

@mdwn mdwn May 5, 2023

Choose a reason for hiding this comment

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

Truth be told I was taking a cue from AdditionalReadyEvents, but thinking on it it should probably be this way so that the OSS process can overwrite any conflicting instances with the ones it expects. Not that I expect that to be a frequent occurrence.


if cfg.Auth.Enabled {
process.SetExpectedInstanceRole(types.RoleAuth, AuthIdentityEvent)
}
Expand Down
93 changes: 93 additions & 0 deletions lib/service/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,99 @@ func TestServiceSelfSignedHTTPS(t *testing.T) {
require.FileExists(t, cfg.Proxy.KeyPairs[0].PrivateKey)
}

func TestAdditionalExpectedRoles(t *testing.T) {
tests := []struct {
name string
cfg func() *servicecfg.Config
expectedRoles map[types.SystemRole]string
}{
{
name: "everything enabled",
cfg: func() *servicecfg.Config {
cfg := servicecfg.MakeDefaultConfig()
cfg.DataDir = t.TempDir()
cfg.SetAuthServerAddress(utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"})
cfg.Auth.StorageConfig.Params["path"] = t.TempDir()
cfg.DiagnosticAddr = utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}
cfg.Auth.ListenAddr = utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}

cfg.Auth.Enabled = true
cfg.SSH.Enabled = true
cfg.Proxy.Enabled = true
cfg.Kube.Enabled = true
cfg.Apps.Enabled = true
cfg.Databases.Enabled = true
cfg.WindowsDesktop.Enabled = true
cfg.Discovery.Enabled = true
return cfg
},
expectedRoles: map[types.SystemRole]string{
types.RoleAuth: AuthIdentityEvent,
types.RoleNode: SSHIdentityEvent,
types.RoleKube: KubeIdentityEvent,
types.RoleApp: AppsIdentityEvent,
types.RoleDatabase: DatabasesIdentityEvent,
types.RoleWindowsDesktop: WindowsDesktopIdentityEvent,
types.RoleDiscovery: DiscoveryIdentityEvent,
types.RoleProxy: ProxyIdentityEvent,
},
},
{
name: "everything enabled with additional roles",
cfg: func() *servicecfg.Config {
cfg := servicecfg.MakeDefaultConfig()
cfg.DataDir = t.TempDir()
cfg.SetAuthServerAddress(utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"})
cfg.Auth.StorageConfig.Params["path"] = t.TempDir()
cfg.DiagnosticAddr = utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}
cfg.Auth.ListenAddr = utils.NetAddr{AddrNetwork: "tcp", Addr: "127.0.0.1:0"}

cfg.Auth.Enabled = true
cfg.SSH.Enabled = true
cfg.Proxy.Enabled = true
cfg.Kube.Enabled = true
cfg.Apps.Enabled = true
cfg.Databases.Enabled = true
cfg.WindowsDesktop.Enabled = true
cfg.Discovery.Enabled = true

cfg.AdditionalExpectedRoles = []servicecfg.RoleAndIdentityEvent{
{
Role: types.RoleOkta,
IdentityEvent: "some-identity-event",
},
{
Role: types.RoleBot,
IdentityEvent: "some-other-identity-event",
},
}

return cfg
},
expectedRoles: map[types.SystemRole]string{
types.RoleAuth: AuthIdentityEvent,
types.RoleNode: SSHIdentityEvent,
types.RoleKube: KubeIdentityEvent,
types.RoleApp: AppsIdentityEvent,
types.RoleDatabase: DatabasesIdentityEvent,
types.RoleWindowsDesktop: WindowsDesktopIdentityEvent,
types.RoleDiscovery: DiscoveryIdentityEvent,
types.RoleProxy: ProxyIdentityEvent,
types.RoleOkta: "some-identity-event",
types.RoleBot: "some-other-identity-event",
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
process, err := NewTeleport(test.cfg())
require.NoError(t, err)
require.Equal(t, test.expectedRoles, process.instanceRoles)
})
}
}

func TestMonitor(t *testing.T) {
t.Parallel()
fakeClock := clockwork.NewFakeClock()
Expand Down
12 changes: 12 additions & 0 deletions lib/service/servicecfg/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ type Config struct {
// CircuitBreakerConfig configures the auth client circuit breaker.
CircuitBreakerConfig breaker.Config

// AdditionalExpectedRoles are additional roles to attach to the Teleport instances.
AdditionalExpectedRoles []RoleAndIdentityEvent

// AdditionalReadyEvents are additional events to watch for to consider the Teleport instance ready.
AdditionalReadyEvents []string

Expand Down Expand Up @@ -267,6 +270,15 @@ type Config struct {
authServers []utils.NetAddr
}

// RoleAndIdentityEvent is a role and its corresponding identity event.
type RoleAndIdentityEvent struct {
// Role is a system role.
Role types.SystemRole

// IdentityEvent is the identity event associated with the above role.
IdentityEvent string
}

// JoinParams is a set of extra parameters for joining the auth server.
type JoinParams struct {
Azure AzureJoinParams
Expand Down