From ba2fcd79a39a52765700125552cee29cefe1580b Mon Sep 17 00:00:00 2001 From: Fedor Setrakov Date: Tue, 11 Nov 2025 14:47:52 +0300 Subject: [PATCH] Pull request 2521: AGDNS-3361-imp-web-construction Squashed commit of the following: commit 3de6526c1584939e0474bb125242356d75da68ed Author: f.setrakov Date: Tue Nov 11 11:32:52 2025 +0300 home: imp newTestWeb helper commit b67480db9f8d2ecda48a16e66dda241768f790a5 Author: f.setrakov Date: Mon Nov 10 12:51:59 2025 +0300 home: imp docs, imp tests commit 0699ae709f82da07a1f346fd9fb2bf856a89e859 Author: f.setrakov Date: Fri Nov 7 10:01:56 2025 +0300 home: imp docs, imp tests commit 41bfff532eb2a67e641822e0af0d224e9e78a903 Author: f.setrakov Date: Thu Nov 6 19:31:45 2025 +0300 home: imp web init, imp tests commit fc28f2e5e933c933b1695078ca53a17add7daf40 Author: f.setrakov Date: Thu Nov 6 18:02:04 2025 +0300 home: imp newWeb func --- internal/home/authhttp_internal_test.go | 68 +++-------- internal/home/home.go | 133 +++++++++++++-------- internal/home/home_internal_test.go | 38 ++++++ internal/home/profilehttp_internal_test.go | 44 ++----- internal/home/tls_internal_test.go | 34 +----- internal/home/web.go | 13 +- 6 files changed, 160 insertions(+), 170 deletions(-) diff --git a/internal/home/authhttp_internal_test.go b/internal/home/authhttp_internal_test.go index df8f37ad5..80d6d42a9 100644 --- a/internal/home/authhttp_internal_test.go +++ b/internal/home/authhttp_internal_test.go @@ -325,24 +325,11 @@ func TestAuth_ServeHTTP_firstRun(t *testing.T) { mux := http.NewServeMux() httpReg := aghhttp.NewDefaultRegistrar(mux, mw.wrap) - ctx := testutil.ContextWithTimeout(t, testTimeout) - web, err := initWeb( - ctx, - options{}, - nil, - nil, - testLogger, - nil, - nil, - mux, - agh.EmptyConfigModifier{}, - httpReg, - "", - "", - false, - true, - ) - require.NoError(t, err) + web := newTestWeb(t, &webConfig{ + mux: mux, + httpReg: httpReg, + isFirstRun: true, + }) globalContext.web = web mw.set(web) @@ -499,23 +486,12 @@ func TestAuth_ServeHTTP_auth(t *testing.T) { }) require.NoError(t, err) - ctx := testutil.ContextWithTimeout(t, testTimeout) - web, err := initWeb( - ctx, - options{}, - nil, - nil, - testLogger, - tlsMgr, - auth, - baseMux, - agh.EmptyConfigModifier{}, - httpReg, - "", - "", - false, - false, - ) + web := newTestWeb(t, &webConfig{ + tlsManager: tlsMgr, + auth: auth, + mux: baseMux, + httpReg: httpReg, + }) require.NoError(t, err) globalContext.web = web @@ -659,23 +635,11 @@ func TestAuth_ServeHTTP_logout(t *testing.T) { baseMux := http.NewServeMux() httpReg := aghhttp.NewDefaultRegistrar(baseMux, mw.wrap) - ctx := testutil.ContextWithTimeout(t, testTimeout) - web, err := initWeb( - ctx, - options{}, - nil, - nil, - testLogger, - nil, - auth, - baseMux, - agh.EmptyConfigModifier{}, - httpReg, - "", - "", - false, - false, - ) + web := newTestWeb(t, &webConfig{ + auth: auth, + mux: baseMux, + httpReg: httpReg, + }) require.NoError(t, err) globalContext.web = web diff --git a/internal/home/home.go b/internal/home/home.go index 101919b22..44dc04893 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -570,60 +570,90 @@ func isUpdateEnabled( } } -// initWeb initializes the web module. All arguments must not be nil. -// -// TODO(s.chzhen): Use a configuration structure. -func initWeb( - ctx context.Context, - opts options, - clientBuildFS fs.FS, - upd *updater.Updater, - baseLogger *slog.Logger, - tlsMgr *tlsManager, - auth *auth, - mux *http.ServeMux, - confModifier agh.ConfigModifier, - httpReg aghhttp.Registrar, - workDir string, - confPath string, - isCustomUpdURL bool, - isFirstRun bool, -) (web *webAPI, err error) { - logger := baseLogger.With(slogutil.KeyPrefix, "webapi") +// webConfig is a configuration structure for webAPI. +type webConfig struct { + // opts are used to determine if update is enabled. + opts options + + // clientBuildFS is used for initializing client FS. If opts.localFrontend + // is false, then this field must not be nil. + clientBuildFS fs.FS + + // updater is used for handling updates. It must not be nil. + updater *updater.Updater + + // baseLogger is used for logging init process and for logging inside web + // api. It must not be nil. + baseLogger *slog.Logger + + // tlsManager contains the current configuration and state of TLS + // encryption. It must not be nil. + tlsManager *tlsManager + + // auth stores web user information and handles authentication. It must not + // be nil. + auth *auth + + // mux is the default *http.ServeMux, the same as [globalContext.mux]. It + // must not be nil. + mux *http.ServeMux + + // configModifier is used to update the global configuration. + configModifier agh.ConfigModifier + + // httpReg registers HTTP handlers. It must not be nil. + httpReg aghhttp.Registrar + + // workDir is a base working directory. + workDir string + + // confPath is a config path. + confPath string + + // isCustomUpdURL defines if updater should use custom url. + isCustomUpdURL bool + + // isFirstRun defines if current run is the first run. + isFirstRun bool +} + +// newWeb initializes the web module. conf must not be nil. +func newWeb(ctx context.Context, conf *webConfig) (web *webAPI, err error) { + logger := conf.baseLogger.With(slogutil.KeyPrefix, "webapi") webPort := suggestedWebPort(ctx, logger) var clientFS fs.FS - if opts.localFrontend { + if conf.opts.localFrontend { logger.WarnContext(ctx, "using local frontend files") clientFS = os.DirFS("build/static") } else { - clientFS, err = fs.Sub(clientBuildFS, "build/static") + clientFS, err = fs.Sub(conf.clientBuildFS, "build/static") if err != nil { return nil, fmt.Errorf("getting embedded client subdir: %w", err) } } - disableUpdate := !isUpdateEnabled(ctx, baseLogger, &opts, isCustomUpdURL) + disableUpdate := !isUpdateEnabled(ctx, conf.baseLogger, &conf.opts, conf.isCustomUpdURL) - webConf := &webConfig{ + webConf := &webAPIConfig{ CommandConstructor: executil.SystemCommandConstructor{}, - updater: upd, + updater: conf.updater, logger: logger, - baseLogger: baseLogger, - confModifier: confModifier, - httpReg: httpReg, - tlsManager: tlsMgr, - auth: auth, - mux: mux, + baseLogger: conf.baseLogger, + confModifier: conf.configModifier, + httpReg: conf.httpReg, + tlsManager: conf.tlsManager, + auth: conf.auth, + mux: conf.mux, clientFS: clientFS, BindAddr: config.HTTPConfig.Address, - workDir: workDir, - confPath: confPath, + workDir: conf.workDir, + confPath: conf.confPath, ReadTimeout: readTimeout, ReadHeaderTimeout: readHdrTimeout, @@ -631,9 +661,9 @@ func initWeb( defaultWebPort: webPort, - firstRun: isFirstRun, + firstRun: conf.isFirstRun, disableUpdate: disableUpdate, - runningAsService: opts.runningAsService, + runningAsService: conf.opts.runningAsService, serveHTTP3: config.DNS.ServeHTTP3, } @@ -725,22 +755,23 @@ func run( confModifier.setAuth(auth) - web, err := initWeb( - ctx, - opts, - clientBuildFS, - upd, - slogLogger, - tlsMgr, - auth, - mux, - confModifier, - httpReg, - workDir, - confPath, - isCustomURL, - isFirstRun, - ) + conf := &webConfig{ + clientBuildFS: clientBuildFS, + updater: upd, + opts: opts, + baseLogger: slogLogger, + tlsManager: tlsMgr, + auth: auth, + mux: mux, + configModifier: confModifier, + httpReg: httpReg, + workDir: workDir, + confPath: confPath, + isCustomUpdURL: isCustomURL, + isFirstRun: isFirstRun, + } + + web, err := newWeb(ctx, conf) fatalOnError(err) mw.set(web) diff --git a/internal/home/home_internal_test.go b/internal/home/home_internal_test.go index 0762cb671..e6d3b913e 100644 --- a/internal/home/home_internal_test.go +++ b/internal/home/home_internal_test.go @@ -1,14 +1,52 @@ package home import ( + "cmp" + "net/http" "testing" + "github.com/AdguardTeam/AdGuardHome/internal/agh" + "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/testutil" + "github.com/stretchr/testify/require" ) +// testLogger is a common logger for tests. var testLogger = slogutil.NewDiscardLogger() +// newTestWeb is a helper that creates new webAPI and fills it's config with +// given values. If conf is nil, the default configuration will be used. +func newTestWeb( + tb testing.TB, + conf *webConfig, +) (web *webAPI) { + tb.Helper() + + ctx := testutil.ContextWithTimeout(tb, testTimeout) + conf = cmp.Or(conf, &webConfig{}) + + web, err := newWeb(ctx, &webConfig{ + clientBuildFS: conf.clientBuildFS, + updater: conf.updater, + opts: conf.opts, + baseLogger: testLogger, + tlsManager: conf.tlsManager, + auth: conf.auth, + mux: cmp.Or(conf.mux, http.NewServeMux()), + configModifier: cmp.Or[agh.ConfigModifier](conf.configModifier, &agh.EmptyConfigModifier{}), + httpReg: cmp.Or[aghhttp.Registrar](conf.httpReg, &aghhttp.EmptyRegistrar{}), + workDir: conf.workDir, + confPath: conf.confPath, + isCustomUpdURL: conf.isCustomUpdURL, + isFirstRun: conf.isFirstRun, + }) + + require.NoError(tb, err) + + return web +} + func TestMain(m *testing.M) { initCmdLineOpts() testutil.DiscardLogOutput(m) diff --git a/internal/home/profilehttp_internal_test.go b/internal/home/profilehttp_internal_test.go index f7669aaf1..3afe61b5e 100644 --- a/internal/home/profilehttp_internal_test.go +++ b/internal/home/profilehttp_internal_test.go @@ -79,22 +79,11 @@ func TestWeb_HandleGetProfile(t *testing.T) { }) require.NoError(t, err) - web, err := initWeb( - testutil.ContextWithTimeout(t, testTimeout), - options{}, - nil, - nil, - testLogger, - tlsMgr, - auth, - baseMux, - agh.EmptyConfigModifier{}, - aghhttp.EmptyRegistrar{}, - "", - "", - false, - false, - ) + web := newTestWeb(t, &webConfig{ + tlsManager: tlsMgr, + auth: auth, + mux: baseMux, + }) require.NoError(t, err) globalContext.web = web @@ -143,24 +132,11 @@ func TestWeb_HandlePutProfile(t *testing.T) { OnApply: func(_ context.Context) { isConfigChanged = true }, } - ctx := testutil.ContextWithTimeout(t, testTimeout) - web, err := initWeb( - ctx, - options{}, - nil, - nil, - testLogger, - nil, - nil, - mux, - confModifier, - httpReg, - "", - "", - false, - false, - ) - require.NoError(t, err) + web := newTestWeb(t, &webConfig{ + mux: mux, + configModifier: confModifier, + httpReg: httpReg, + }) globalContext.web = web mw.set(web) diff --git a/internal/home/tls_internal_test.go b/internal/home/tls_internal_test.go index 432d09dd1..8478850c2 100644 --- a/internal/home/tls_internal_test.go +++ b/internal/home/tls_internal_test.go @@ -22,7 +22,6 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/agh" "github.com/AdguardTeam/AdGuardHome/internal/aghalg" - "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/AdGuardHome/internal/client" "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" "github.com/AdguardTeam/golibs/testutil" @@ -292,31 +291,6 @@ func assertCertSerialNumber(tb testing.TB, conf *tlsConfigSettings, wantSN int64 assert.Equal(tb, wantSN, cert.Leaf.SerialNumber.Int64()) } -// initEmptyWeb returns an initialized *webAPI with zero values and no-op mocks. -func initEmptyWeb(tb testing.TB) (web *webAPI) { - tb.Helper() - - web, err := initWeb( - testutil.ContextWithTimeout(tb, testTimeout), - options{}, - nil, - nil, - testLogger, - nil, - nil, - http.NewServeMux(), - agh.EmptyConfigModifier{}, - aghhttp.EmptyRegistrar{}, - "", - "", - false, - false, - ) - require.NoError(tb, err) - - return web -} - func TestTLSManager_Reload(t *testing.T) { storeGlobals(t) @@ -363,7 +337,7 @@ func TestTLSManager_Reload(t *testing.T) { }) require.NoError(t, err) - web := initEmptyWeb(t) + web := newTestWeb(t, &webConfig{}) m.setWebAPI(web) conf := m.config() @@ -431,7 +405,7 @@ func TestValidateTLSSettings(t *testing.T) { }) require.NoError(t, err) - web := initEmptyWeb(t) + web := newTestWeb(t, &webConfig{}) m.setWebAPI(web) tcpLn, err := net.Listen("tcp", ":0") @@ -531,7 +505,7 @@ func TestTLSManager_HandleTLSValidate(t *testing.T) { }) require.NoError(t, err) - web := initEmptyWeb(t) + web := newTestWeb(t, &webConfig{}) m.setWebAPI(web) setts := &tlsConfigSettingsExt{ @@ -620,7 +594,7 @@ func TestTLSManager_HandleTLSConfigure(t *testing.T) { }) require.NoError(t, err) - web := initEmptyWeb(t) + web := newTestWeb(t, &webConfig{}) m.setWebAPI(web) conf := m.config() diff --git a/internal/home/web.go b/internal/home/web.go index 137619013..45c10b6bc 100644 --- a/internal/home/web.go +++ b/internal/home/web.go @@ -40,10 +40,13 @@ const ( writeTimeout = 5 * time.Minute ) -type webConfig struct { +// webAPIConfig is a configuration structure for webAPI. +type webAPIConfig struct { // CommandConstructor is used to run external commands. It must not be nil. CommandConstructor executil.CommandConstructor + // updater is used for updating AdGuard home. If disableUpdate is set to + // false, it must not be nil. updater *updater.Updater // logger is a slog logger used in webAPI. It must not be nil. @@ -71,6 +74,7 @@ type webConfig struct { // must not be nil. mux *http.ServeMux + // clientFS is used to initialize file server. It must not be nil. clientFS fs.FS // BindAddr is the binding address with port for plain HTTP web interface. @@ -97,6 +101,7 @@ type webConfig struct { // defaultWebPort is the suggested default HTTP port for the install wizard. defaultWebPort uint16 + // firstRun, if true, tells AdGuard Home to register install handlers. firstRun bool // disableUpdate, if true, tells AdGuard Home to not check for updates. @@ -106,6 +111,7 @@ type webConfig struct { // service runner. runningAsService bool + // serveHTTP3, if true, tells AdGuard Home to start HTTP3 server. serveHTTP3 bool } @@ -127,7 +133,7 @@ type httpsServer struct { // webAPI is the web UI and API server. type webAPI struct { - conf *webConfig + conf *webAPIConfig // confModifier is used to update the global configuration. confModifier agh.ConfigModifier @@ -167,7 +173,7 @@ type webAPI struct { // valid. // // TODO(a.garipov): Return a proper error. -func newWebAPI(ctx context.Context, conf *webConfig) (w *webAPI) { +func newWebAPI(ctx context.Context, conf *webAPIConfig) (w *webAPI) { conf.logger.InfoContext(ctx, "initializing") w = &webAPI{ @@ -408,6 +414,7 @@ func (web *webAPI) waitForTLSReady() (ok bool) { return true } +// mustStartHTTP3 initializes and starts HTTP3 server. func (web *webAPI) mustStartHTTP3(ctx context.Context, address string) { defer slogutil.RecoverAndExit(ctx, web.logger, osutil.ExitCodeFailure)