Skip to content

Commit

Permalink
Add GPO list daemon timeout option (#1097)
Browse files Browse the repository at this point in the history
Added a `--gpo-list-timeout` flag and a `gpo_list_timeout` configuration parameter on the adsys daemon.

By default, the GPO list timeout is fixed to 10 seconds. If the GPO list python script takes more than 10 seconds it's killed by the timeout.
However, there might be scenarios where 10 seconds might not be enough. In our use case, the GPO list procedure finishes around 15 seconds in average.

Big thank you to dangerousplay!
  • Loading branch information
didrocks authored Sep 13, 2024
2 parents f693197 + 73212e1 commit fb64f0b
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 8 deletions.
12 changes: 9 additions & 3 deletions cmd/adsysd/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ type daemonConfig struct {
SystemUnitDir string `mapstructure:"systemunit_dir"`
GlobalTrustDir string `mapstructure:"global_trust_dir"`

AdBackend string `mapstructure:"ad_backend"`
SSSdConfig sss.Config `mapstructure:"sssd"`
WinbindConfig winbind.Config `mapstructure:"winbind"`
AdBackend string `mapstructure:"ad_backend"`
SSSdConfig sss.Config `mapstructure:"sssd"`
WinbindConfig winbind.Config `mapstructure:"winbind"`
GpoListTimeout int `mapstructure:"gpo_list_timeout"`

ServiceTimeout int `mapstructure:"service_timeout"`
}
Expand Down Expand Up @@ -125,6 +126,7 @@ func New() *App {
adsysservice.WithADBackend(a.config.AdBackend),
adsysservice.WithSSSConfig(a.config.SSSdConfig),
adsysservice.WithWinbindConfig(a.config.WinbindConfig),
adsysservice.WithGpoListTimeout(time.Second*time.Duration(a.config.GpoListTimeout)),
)
if err != nil {
close(a.ready)
Expand Down Expand Up @@ -163,6 +165,10 @@ func New() *App {
err = a.viper.BindPFlag("service_timeout", a.rootCmd.PersistentFlags().Lookup("timeout"))
decorate.LogOnError(&err)

a.rootCmd.PersistentFlags().IntP("gpo-list-timeout", "", consts.DefaultGpoListTimeout, gotext.Get("time in seconds for the GPO list. 0 for no timeout."))
err = a.viper.BindPFlag("gpo_list_timeout", a.rootCmd.PersistentFlags().Lookup("gpo-list-timeout"))
decorate.LogOnError(&err)

a.rootCmd.PersistentFlags().StringP("ad-backend", "", "sssd", gotext.Get("Active Directory authentication backend"))
err = a.viper.BindPFlag("ad_backend", a.rootCmd.PersistentFlags().Lookup("ad-backend"))
decorate.LogOnError(&err)
Expand Down
3 changes: 3 additions & 0 deletions conf.example/adsys.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,6 @@ detect_cached_ticket: false

# Client only configuration
client_timeout: 60

# GPO List timeout
gpo_list_timeout: 10
1 change: 1 addition & 0 deletions docs/.custom_wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ FQDN
GDM
gdm
GPL
gpo
GPO
gpolist
GPOs
Expand Down
6 changes: 6 additions & 0 deletions docs/reference/adsys-daemon.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,12 @@ A custom domain can be used to override the C API call that ADSys executes to de

A custom domain controller can be used to override the C API call that ADSys executes to determine the AD controller FQDN -- which is returned by `wbinfo --dsgetdcname domain.com` (e.g. `adc.example.com`).

### GPO configuration:

* **gpo_list_timeout**

Maximum time in seconds for the GPO list to finish otherwise the GPO list is aborted. This can be overridden by the `--gpo-list-timeout` option. Defaults to 10 seconds.

### Client only configuration:**

* **client_timeout**
Expand Down
2 changes: 1 addition & 1 deletion internal/ad/ad.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func (ad *AD) GetPolicies(ctx context.Context, objectName string, objectClass Ob
args := append([]string{}, ad.gpoListCmd...) // Copy gpoListCmd to prevent data race
scriptArgs := []string{"--objectclass", string(objectClass), adServerFQDN, objectName}
cmdArgs := append(args, scriptArgs...)
cmdCtx, cancel := context.WithTimeout(ctx, time.Second*10)
cmdCtx, cancel := context.WithTimeout(ctx, ad.gpoListTimeout)
defer cancel()
log.Debugf(ctx, "Getting gpo list with arguments: %q", strings.Join(scriptArgs, " "))
// #nosec G204 - cmdArgs is under our control (python embedded script or mock for tests)
Expand Down
12 changes: 11 additions & 1 deletion internal/adsysservice/adsysservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ type options struct {
systemUnitDir string
globalTrustDir string
adBackend string
gpoListTimeout time.Duration
sssConfig sss.Config
winbindConfig winbind.Config
authorizer authorizerer
Expand Down Expand Up @@ -187,6 +188,14 @@ func WithWinbindConfig(c winbind.Config) func(o *options) error {
}
}

// WithGpoListTimeout specifies the timeout for the gpo list

Check failure on line 191 in internal/adsysservice/adsysservice.go

View workflow job for this annotation

GitHub Actions / Code sanity

Comment should end in a period (godot)
func WithGpoListTimeout(t time.Duration) func(o *options) error {
return func(o *options) error {
o.gpoListTimeout = t
return nil
}
}

// New returns a new instance of an AD service.
// If url or domain is empty, we load the missing parameters from sssd.conf, taking first
// domain in the list if not provided.
Expand Down Expand Up @@ -241,7 +250,8 @@ func New(ctx context.Context, opts ...option) (s *Service, err error) {
if args.runDir != "" {
adOptions = append(adOptions, ad.WithRunDir(args.runDir))
}
adOptions = append(adOptions, ad.WithGpoListTimeout(consts.DefaultGpoListTimeout))

adOptions = append(adOptions, ad.WithGpoListTimeout(args.gpoListTimeout))

hostname, err := os.Hostname()
if err != nil {
Expand Down
4 changes: 1 addition & 3 deletions internal/consts/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
package consts

import (
"time"

log "github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -41,7 +39,7 @@ const (
DefaultServiceTimeout = 120

// DefaultGpoListTimeout is the default time to wait for the GPO list subcommand to finish.
DefaultGpoListTimeout = 10 * time.Second
DefaultGpoListTimeout = 10

// DistroID is the distro ID which can be overridden at build time.
DistroID = "Ubuntu"
Expand Down

0 comments on commit fb64f0b

Please sign in to comment.