From 143616ca6e302aff0e5374d34c1d85fbd11ab544 Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen <s.chzhen@adguard.com> Date: Thu, 23 Mar 2023 13:46:57 +0300 Subject: [PATCH] Pull request 1736: 4299-querylog-stats-api Merge in DNS/adguard-home from 4299-querylog-stats-api to master Updates #1717. Updates #4299. Squashed commit of the following: commit 5b706b7997a536bc4fd2c532fb89ca5ab3536848 Merge: 48b62b0f 306c1983 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 22 13:53:09 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit 48b62b0f1882f1ad120c6cdd90cd7dd8cb8a7738 Author: Vladislav Abdulmyanov <v.abdulmyanov@adguard.com> Date: Wed Mar 22 12:25:04 2023 +0200 client: fix styles, add titles and descrs commit 97e31cff70d05b51bd0e5ea2d20e8e7a251a7e41 Author: Vladislav Abdulmyanov <v.abdulmyanov@adguard.com> Date: Tue Mar 21 18:38:12 2023 +0200 client: add ignored domains for querylog commit 24d75c4376382205ae6b8f731b1cd23d517772c9 Author: Vladislav Abdulmyanov <v.abdulmyanov@adguard.com> Date: Tue Mar 21 18:21:13 2023 +0200 client: add ignore domains for stats commit eefc3891d01f90af79fdac9ba8eea06d4d54a0bc Merge: 978675ea 1daabb97 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Mar 21 10:53:35 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit 978675ea2c07bf248b4c8f26ebdf78cf59a12ef5 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Mar 21 10:53:11 2023 +0300 openapi: fix chlog commit 2ed33007aade115d38b0ca582206cc10678b084c Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Mar 20 17:49:07 2023 +0300 home: fix tests commit 6af11520c164553ee9fce8f214ea169672188d7e Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Mar 20 17:40:16 2023 +0300 home: fix typo commit 56acdfde5b1ee8d16b232c1293b91affbe319ad1 Merge: 319da34d 48431f8b Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Mar 20 17:32:58 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit 319da34de41ec84310b23bba2ad79c8a3a4c14ff Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Mar 3 17:34:38 2023 +0300 querylog: fix docs commit d5a8f24d5b336e7bdbbca18069f6ede8c96bcc2c Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Mar 3 11:42:00 2023 +0300 stats: fix docs commit e0cbfc1c4078180a05835ce7587e9f45484adc81 Merge: 4743c810 012e5beb Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 1 18:45:17 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit 4743c81038052b9e0ca29ae5f1565021d36ca1ef Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 1 18:14:16 2023 +0300 all: imp code; fix time conversion commit 34310cffd7e331d098c535590245387051674fa8 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 1 12:34:11 2023 +0300 chlog: restore order commit cadd864a66655242948f1cb16e6d4945c0235d7e Merge: 2f3e25be bb226434 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 1 12:26:06 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit 2f3e25bee56d2c6ddcf4aa2fc6a1dc51ed9b06e1 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Mar 1 12:25:14 2023 +0300 all: fix fmt commit d54022baa6c8a3d0d3c308a9b6b1a6a9dc6ac7b6 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Feb 28 16:16:40 2023 +0300 all: imp code; fix chlog commit df22de91f59a51194c55e7bcbe5bc3fcc60cb8e3 Merge: e1ea4797 a772212d Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Feb 27 17:24:09 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit e1ea4797af974c36f06683ffc6eaaae917921a43 Merge: d7db0a5a bb80a7c2 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Feb 27 17:23:20 2023 +0300 Merge branch 'master' into 4299-querylog-stats-api commit d7db0a5af1e1f49f6174c1c42e6d9306f2381d16 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Feb 27 17:12:20 2023 +0300 all: imp docs ... and 15 more commits --- CHANGELOG.md | 45 +++++- client/src/__locales/en.json | 4 + client/src/actions/queryLogs.js | 2 +- client/src/actions/stats.js | 2 +- client/src/api/Api.js | 20 +-- .../components/Settings/LogsConfig/Form.js | 36 ++++- .../components/Settings/LogsConfig/index.js | 10 +- .../components/Settings/StatsConfig/Form.js | 48 +++--- .../components/Settings/StatsConfig/index.js | 15 +- client/src/components/Settings/index.js | 6 + client/src/helpers/constants.js | 9 +- client/src/reducers/stats.js | 4 +- internal/aghnet/hostgen.go | 29 ++++ internal/home/config.go | 38 +++-- internal/home/dns.go | 37 +---- internal/home/upgrade.go | 44 ++++- internal/home/upgrade_test.go | 95 +++++++++++ internal/querylog/http.go | 131 ++++++++++++++- internal/querylog/qlog.go | 17 ++ internal/querylog/qlog_test.go | 15 +- internal/querylog/querylog.go | 18 +-- internal/querylog/search_test.go | 3 +- internal/stats/http.go | 104 +++++++++++- internal/stats/http_test.go | 152 ++++++++++++++++++ internal/stats/stats.go | 60 ++++--- internal/stats/stats_internal_test.go | 7 +- internal/stats/stats_test.go | 11 +- openapi/CHANGELOG.md | 65 ++++++++ openapi/openapi.yaml | 125 ++++++++++++++ 29 files changed, 995 insertions(+), 157 deletions(-) create mode 100644 internal/stats/http_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 61f8706a..39f44fc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,12 @@ NOTE: Add new changes BELOW THIS COMMENT. ### Added +- Two new HTTP APIs, `PUT /control/stats/config/update` and `GET + control/stats/config`, which can be used to set and receive the query log + configuration. See openapi/openapi.yaml for the full description. +- Two new HTTP APIs, `PUT /control/querylog/config/update` and `GET + control/querylog/config`, which can be used to set and receive the statistics + configuration. See openapi/openapi.yaml for the full description. - The ability to set custom IP for EDNS Client Subnet by using the DNS-server configuration section on the DNS settings page in the UI ([#1472]). - The ability to manage safesearch for each service by using the new @@ -37,8 +43,26 @@ NOTE: Add new changes BELOW THIS COMMENT. #### Configuration Changes -In this release, the schema version has changed from 17 to 19. +In this release, the schema version has changed from 17 to 20. +- Property `statistics.interval`, which in schema versions 19 and earlier used + to be an integer number of days, is now a string with a human-readable + duration: + + ```yaml + # BEFORE: + 'statistics': + # … + 'interval': 1 + + # AFTER: + 'statistics': + # … + 'interval': '24h' + ``` + + To rollback this change, convert the property back into days and change the + `schema_version` back to `19`. - The `dns.safesearch_enabled` field has been replaced with `safe_search` object containing per-service settings. - The `clients.persistent.safesearch_enabled` field has been replaced with @@ -64,6 +88,23 @@ In this release, the schema version has changed from 17 to 19. client's specific `clients.persistent.safesearch` and then change the `schema_version` back to `17`. +### Deprecated + +- The `GET /control/stats_info` HTTP API; use the new `GET + /control/stats/config` API instead. + + **NOTE:** If interval is custom then it will be equal to `90` days for + compatibility reasons. See openapi/openapi.yaml and `openapi/CHANGELOG.md`. +- The `POST /control/stats_config` HTTP API; use the new `PUT + /control/stats/config/update` API instead. +- The `GET /control/querylog_info` HTTP API; use the new `GET + /control/querylog/config` API instead. + + **NOTE:** If interval is custom then it will be equal to `90` days for + compatibility reasons. See openapi/openapi.yaml and `openapi/CHANGELOG.md`. +- The `POST /control/querylog_config` HTTP API; use the new `PUT + /control/querylog/config/update` API instead. + ### Fixed - Panic caused by empty top-level domain name label in `/etc/hosts` files @@ -103,8 +144,6 @@ See also the [v0.107.26 GitHub milestone][ms-v0.107.26]. #### Configuration Changes -In this release, the schema version has changed from 16 to 17. - - Property `edns_client_subnet`, which in schema versions 16 and earlier used to be a part of the `dns` object, is now part of the `dns.edns_client_subnet` object: diff --git a/client/src/__locales/en.json b/client/src/__locales/en.json index 5ccd771b..902fc4d3 100644 --- a/client/src/__locales/en.json +++ b/client/src/__locales/en.json @@ -525,6 +525,10 @@ "statistics_retention_confirm": "Are you sure you want to change statistics retention? If you decrease the interval value, some data will be lost", "statistics_cleared": "Statistics successfully cleared", "statistics_enable": "Enable statistics", + "ignore_domains": "Ignored domains (separated by newline)", + "ignore_domains_title": "Ignored domains", + "ignore_domains_desc_stats": "Queries for these domains are not written to the statistics", + "ignore_domains_desc_query": "Queries for these domains are not written to the query log", "interval_hours": "{{count}} hour", "interval_hours_plural": "{{count}} hours", "filters_configuration": "Filters configuration", diff --git a/client/src/actions/queryLogs.js b/client/src/actions/queryLogs.js index 99da2cb0..e07c6fae 100644 --- a/client/src/actions/queryLogs.js +++ b/client/src/actions/queryLogs.js @@ -177,7 +177,7 @@ export const getLogsConfigSuccess = createAction('GET_LOGS_CONFIG_SUCCESS'); export const getLogsConfig = () => async (dispatch) => { dispatch(getLogsConfigRequest()); try { - const data = await apiClient.getQueryLogInfo(); + const data = await apiClient.getQueryLogConfig(); dispatch(getLogsConfigSuccess(data)); } catch (error) { dispatch(addErrorToast({ error })); diff --git a/client/src/actions/stats.js b/client/src/actions/stats.js index d3948efa..0e5b416e 100644 --- a/client/src/actions/stats.js +++ b/client/src/actions/stats.js @@ -13,7 +13,7 @@ export const getStatsConfigSuccess = createAction('GET_STATS_CONFIG_SUCCESS'); export const getStatsConfig = () => async (dispatch) => { dispatch(getStatsConfigRequest()); try { - const data = await apiClient.getStatsInfo(); + const data = await apiClient.getStatsConfig(); dispatch(getStatsConfigSuccess(data)); } catch (error) { dispatch(addErrorToast({ error })); diff --git a/client/src/api/Api.js b/client/src/api/Api.js index d984bbb8..60f1faad 100644 --- a/client/src/api/Api.js +++ b/client/src/api/Api.js @@ -497,9 +497,9 @@ class Api { // Settings for statistics GET_STATS = { path: 'stats', method: 'GET' }; - STATS_INFO = { path: 'stats_info', method: 'GET' }; + GET_STATS_CONFIG = { path: 'stats/config', method: 'GET' }; - STATS_CONFIG = { path: 'stats_config', method: 'POST' }; + UPDATE_STATS_CONFIG = { path: 'stats/config/update', method: 'PUT' }; STATS_RESET = { path: 'stats_reset', method: 'POST' }; @@ -508,13 +508,13 @@ class Api { return this.makeRequest(path, method); } - getStatsInfo() { - const { path, method } = this.STATS_INFO; + getStatsConfig() { + const { path, method } = this.GET_STATS_CONFIG; return this.makeRequest(path, method); } setStatsConfig(data) { - const { path, method } = this.STATS_CONFIG; + const { path, method } = this.UPDATE_STATS_CONFIG; const config = { data, }; @@ -529,9 +529,9 @@ class Api { // Query log GET_QUERY_LOG = { path: 'querylog', method: 'GET' }; - QUERY_LOG_CONFIG = { path: 'querylog_config', method: 'POST' }; + UPDATE_QUERY_LOG_CONFIG = { path: 'querylog/config/update', method: 'PUT' }; - QUERY_LOG_INFO = { path: 'querylog_info', method: 'GET' }; + GET_QUERY_LOG_CONFIG = { path: 'querylog/config', method: 'GET' }; QUERY_LOG_CLEAR = { path: 'querylog_clear', method: 'POST' }; @@ -543,13 +543,13 @@ class Api { return this.makeRequest(url, method); } - getQueryLogInfo() { - const { path, method } = this.QUERY_LOG_INFO; + getQueryLogConfig() { + const { path, method } = this.GET_QUERY_LOG_CONFIG; return this.makeRequest(path, method); } setQueryLogConfig(data) { - const { path, method } = this.QUERY_LOG_CONFIG; + const { path, method } = this.UPDATE_QUERY_LOG_CONFIG; const config = { data, }; diff --git a/client/src/components/Settings/LogsConfig/Form.js b/client/src/components/Settings/LogsConfig/Form.js index 8db3f18d..b29b974e 100644 --- a/client/src/components/Settings/LogsConfig/Form.js +++ b/client/src/components/Settings/LogsConfig/Form.js @@ -4,18 +4,28 @@ import { Field, reduxForm } from 'redux-form'; import { Trans, withTranslation } from 'react-i18next'; import flow from 'lodash/flow'; -import { CheckboxField, renderRadioField, toFloatNumber } from '../../../helpers/form'; -import { FORM_NAME, QUERY_LOG_INTERVALS_DAYS } from '../../../helpers/constants'; +import { + CheckboxField, + renderRadioField, + toFloatNumber, + renderTextareaField, +} from '../../../helpers/form'; +import { + FORM_NAME, + QUERY_LOG_INTERVALS_DAYS, + HOUR, + DAY, +} from '../../../helpers/constants'; import '../FormButton.css'; const getIntervalTitle = (interval, t) => { switch (interval) { - case 0.25: + case 6 * HOUR: return t('interval_6_hour'); - case 1: + case DAY: return t('interval_24_hour'); default: - return t('interval_days', { count: interval }); + return t('interval_days', { count: interval / DAY }); } }; @@ -66,6 +76,22 @@ const Form = (props) => { {getIntervalFields(processing, t, toFloatNumber)} </div> </div> + <label className="form__label form__label--with-desc"> + <Trans>ignore_domains_title</Trans> + </label> + <div className="form__desc form__desc--top"> + <Trans>ignore_domains_desc_query</Trans> + </div> + <div className="form__group form__group--settings"> + <Field + name="ignored" + type="textarea" + className="form-control form-control--textarea font-monospace text-input" + component={renderTextareaField} + placeholder={t('ignore_domains')} + disabled={processing} + /> + </div> <div className="mt-5"> <button type="submit" diff --git a/client/src/components/Settings/LogsConfig/index.js b/client/src/components/Settings/LogsConfig/index.js index 8e837412..146a77b0 100644 --- a/client/src/components/Settings/LogsConfig/index.js +++ b/client/src/components/Settings/LogsConfig/index.js @@ -10,13 +10,15 @@ class LogsConfig extends Component { const { t, interval: prevInterval } = this.props; const { interval } = values; + const data = { ...values, ignored: values.ignored ? values.ignored.split('\n') : [] }; + if (interval !== prevInterval) { // eslint-disable-next-line no-alert if (window.confirm(t('query_log_retention_confirm'))) { - this.props.setLogsConfig(values); + this.props.setLogsConfig(data); } } else { - this.props.setLogsConfig(values); + this.props.setLogsConfig(data); } }; @@ -30,7 +32,7 @@ class LogsConfig extends Component { render() { const { - t, enabled, interval, processing, processingClear, anonymize_client_ip, + t, enabled, interval, processing, processingClear, anonymize_client_ip, ignored, } = this.props; return ( @@ -45,6 +47,7 @@ class LogsConfig extends Component { enabled, interval, anonymize_client_ip, + ignored: ignored.join('\n'), }} onSubmit={this.handleFormSubmit} processing={processing} @@ -62,6 +65,7 @@ LogsConfig.propTypes = { enabled: PropTypes.bool.isRequired, anonymize_client_ip: PropTypes.bool.isRequired, processing: PropTypes.bool.isRequired, + ignored: PropTypes.array.isRequired, processingClear: PropTypes.bool.isRequired, setLogsConfig: PropTypes.func.isRequired, clearLogs: PropTypes.func.isRequired, diff --git a/client/src/components/Settings/StatsConfig/Form.js b/client/src/components/Settings/StatsConfig/Form.js index cf8da0ac..e9cd02fd 100644 --- a/client/src/components/Settings/StatsConfig/Form.js +++ b/client/src/components/Settings/StatsConfig/Form.js @@ -4,23 +4,31 @@ import { Field, reduxForm } from 'redux-form'; import { Trans, withTranslation } from 'react-i18next'; import flow from 'lodash/flow'; -import { renderRadioField, toNumber, CheckboxField } from '../../../helpers/form'; -import { FORM_NAME, STATS_INTERVALS_DAYS, DISABLED_STATS_INTERVAL } from '../../../helpers/constants'; +import { + renderRadioField, + toNumber, + CheckboxField, + renderTextareaField, +} from '../../../helpers/form'; +import { + FORM_NAME, + STATS_INTERVALS_DAYS, + DAY, +} from '../../../helpers/constants'; import '../FormButton.css'; -const getIntervalTitle = (interval, t) => { - switch (interval) { +const getIntervalTitle = (intervalMs, t) => { + switch (intervalMs / DAY) { case 1: return t('interval_24_hour'); default: - return t('interval_days', { count: interval }); + return t('interval_days', { count: intervalMs / DAY }); } }; const Form = (props) => { const { handleSubmit, - change, processing, submitting, invalid, @@ -38,13 +46,6 @@ const Form = (props) => { component={CheckboxField} placeholder={t('statistics_enable')} disabled={processing} - onChange={(event) => { - if (event.target.checked) { - change('interval', STATS_INTERVALS_DAYS[0]); - } else { - change('interval', DISABLED_STATS_INTERVAL); - } - }} /> </div> <label className="form__label form__label--with-desc"> @@ -65,15 +66,26 @@ const Form = (props) => { placeholder={getIntervalTitle(interval, t)} normalize={toNumber} disabled={processing} - onChange={(event) => { - if (event.target.checked) { - change('enabled', true); - } - }} /> ))} </div> </div> + <label className="form__label form__label--with-desc"> + <Trans>ignore_domains_title</Trans> + </label> + <div className="form__desc form__desc--top"> + <Trans>ignore_domains_desc_stats</Trans> + </div> + <div className="form__group form__group--settings"> + <Field + name="ignored" + type="textarea" + className="form-control form-control--textarea font-monospace text-input" + component={renderTextareaField} + placeholder={t('ignore_domains')} + disabled={processing} + /> + </div> <div className="mt-5"> <button type="submit" diff --git a/client/src/components/Settings/StatsConfig/index.js b/client/src/components/Settings/StatsConfig/index.js index 06f0179a..83807afa 100644 --- a/client/src/components/Settings/StatsConfig/index.js +++ b/client/src/components/Settings/StatsConfig/index.js @@ -6,9 +6,13 @@ import Card from '../../ui/Card'; import Form from './Form'; class StatsConfig extends Component { - handleFormSubmit = (values) => { + handleFormSubmit = ({ enabled, interval, ignored }) => { const { t, interval: prevInterval } = this.props; - const config = { interval: values.interval }; + const config = { + enabled, + interval, + ignored: ignored ? ignored.split('\n') : [], + }; if (config.interval < prevInterval) { if (window.confirm(t('statistics_retention_confirm'))) { @@ -29,7 +33,7 @@ class StatsConfig extends Component { render() { const { - t, interval, processing, processingReset, + t, interval, processing, processingReset, ignored, enabled, } = this.props; return ( @@ -42,7 +46,8 @@ class StatsConfig extends Component { <Form initialValues={{ interval, - enabled: !!interval, + enabled, + ignored: ignored.join('\n'), }} onSubmit={this.handleFormSubmit} processing={processing} @@ -57,6 +62,8 @@ class StatsConfig extends Component { StatsConfig.propTypes = { interval: PropTypes.number.isRequired, + ignored: PropTypes.array.isRequired, + enabled: PropTypes.bool.isRequired, processing: PropTypes.bool.isRequired, processingReset: PropTypes.bool.isRequired, setStatsConfig: PropTypes.func.isRequired, diff --git a/client/src/components/Settings/index.js b/client/src/components/Settings/index.js index af89a9be..f4e191c3 100644 --- a/client/src/components/Settings/index.js +++ b/client/src/components/Settings/index.js @@ -98,6 +98,7 @@ class Settings extends Component { <div className="col-md-12"> <LogsConfig enabled={queryLogs.enabled} + ignored={queryLogs.ignored} interval={queryLogs.interval} anonymize_client_ip={queryLogs.anonymize_client_ip} processing={queryLogs.processingSetConfig} @@ -109,6 +110,8 @@ class Settings extends Component { <div className="col-md-12"> <StatsConfig interval={stats.interval} + ignored={stats.ignored} + enabled={stats.enabled} processing={stats.processingSetConfig} processingReset={stats.processingReset} setStatsConfig={setStatsConfig} @@ -139,6 +142,8 @@ Settings.propTypes = { stats: PropTypes.shape({ processingGetConfig: PropTypes.bool, interval: PropTypes.number, + enabled: PropTypes.bool, + ignored: PropTypes.array, processingSetConfig: PropTypes.bool, processingReset: PropTypes.bool, }), @@ -149,6 +154,7 @@ Settings.propTypes = { processingSetConfig: PropTypes.bool, processingClear: PropTypes.bool, processingGetConfig: PropTypes.bool, + ignored: PropTypes.array, }), filtering: PropTypes.shape({ interval: PropTypes.number, diff --git a/client/src/helpers/constants.js b/client/src/helpers/constants.js index 61160d58..acde723f 100644 --- a/client/src/helpers/constants.js +++ b/client/src/helpers/constants.js @@ -211,9 +211,14 @@ export const FILTERED = 'Filtered'; export const NOT_FILTERED = 'NotFiltered'; export const DISABLED_STATS_INTERVAL = 0; -export const STATS_INTERVALS_DAYS = [1, 7, 30, 90]; -export const QUERY_LOG_INTERVALS_DAYS = [0.25, 1, 7, 30, 90]; +export const HOUR = 60 * 60 * 1000; + +export const DAY = HOUR * 24; + +export const STATS_INTERVALS_DAYS = [DAY, DAY * 7, DAY * 30, DAY * 90]; + +export const QUERY_LOG_INTERVALS_DAYS = [HOUR * 6, DAY, DAY * 7, DAY * 30, DAY * 90]; export const FILTERS_INTERVALS_HOURS = [0, 1, 12, 24, 72, 168]; diff --git a/client/src/reducers/stats.js b/client/src/reducers/stats.js index 9c986eac..4f8e25f1 100644 --- a/client/src/reducers/stats.js +++ b/client/src/reducers/stats.js @@ -25,7 +25,7 @@ const stats = handleActions( [actions.getStatsConfigFailure]: (state) => ({ ...state, processingGetConfig: false }), [actions.getStatsConfigSuccess]: (state, { payload }) => ({ ...state, - interval: payload.interval, + ...payload, processingGetConfig: false, }), @@ -33,7 +33,7 @@ const stats = handleActions( [actions.setStatsConfigFailure]: (state) => ({ ...state, processingSetConfig: false }), [actions.setStatsConfigSuccess]: (state, { payload }) => ({ ...state, - interval: payload.interval, + ...payload, processingSetConfig: false, }), diff --git a/internal/aghnet/hostgen.go b/internal/aghnet/hostgen.go index f19001cd..b1421a64 100644 --- a/internal/aghnet/hostgen.go +++ b/internal/aghnet/hostgen.go @@ -1,8 +1,13 @@ package aghnet import ( + "fmt" "net" "strconv" + "strings" + + "github.com/AdguardTeam/golibs/errors" + "github.com/AdguardTeam/golibs/stringutil" ) // The maximum lengths of generated hostnames for different IP versions. @@ -59,3 +64,27 @@ func GenerateHostname(ip net.IP) (hostname string) { return generateIPv6Hostname(ip) } + +// NewDomainNameSet returns nil and error, if list has duplicate or empty +// domain name. Otherwise returns a set, which contains non-FQDN domain names, +// and nil error. +func NewDomainNameSet(list []string) (set *stringutil.Set, err error) { + set = stringutil.NewSet() + + for i, v := range list { + host := strings.ToLower(strings.TrimSuffix(v, ".")) + // TODO(a.garipov): Think about ignoring empty (".") names in the + // future. + if host == "" { + return nil, errors.Error("host name is empty") + } + + if set.Has(host) { + return nil, fmt.Errorf("duplicate host name %q at index %d", host, i) + } + + set.Add(host) + } + + return set, nil +} diff --git a/internal/home/config.go b/internal/home/config.go index 30dcb69d..8eb7694f 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -228,34 +228,32 @@ type tlsConfigSettings struct { } type queryLogConfig struct { + // Ignored is the list of host names, which should not be written to log. + Ignored []string `yaml:"ignored"` + + // Interval is the interval for query log's files rotation. + Interval timeutil.Duration `yaml:"interval"` + + // MemSize is the number of entries kept in memory before they are flushed + // to disk. + MemSize uint32 `yaml:"size_memory"` + // Enabled defines if the query log is enabled. Enabled bool `yaml:"enabled"` // FileEnabled defines, if the query log is written to the file. FileEnabled bool `yaml:"file_enabled"` - - // Interval is the interval for query log's files rotation. - Interval timeutil.Duration `yaml:"interval"` - - // MemSize is the number of entries kept in memory before they are - // flushed to disk. - MemSize uint32 `yaml:"size_memory"` - - // Ignored is the list of host names, which should not be written to - // log. - Ignored []string `yaml:"ignored"` } type statsConfig struct { - // Enabled defines if the statistics are enabled. - Enabled bool `yaml:"enabled"` - - // Interval is the time interval for flushing statistics to the disk in - // days. - Interval uint32 `yaml:"interval"` - // Ignored is the list of host names, which should not be counted. Ignored []string `yaml:"ignored"` + + // Interval is the retention interval for statistics. + Interval timeutil.Duration `yaml:"interval"` + + // Enabled defines if the statistics are enabled. + Enabled bool `yaml:"enabled"` } // config is the global configuration structure. @@ -322,7 +320,7 @@ var config = &configuration{ }, Stats: statsConfig{ Enabled: true, - Interval: 1, + Interval: timeutil.Duration{Duration: 1 * timeutil.Day}, Ignored: []string{}, }, // NOTE: Keep these parameters in sync with the one put into @@ -503,7 +501,7 @@ func (c *configuration) write() (err error) { if Context.stats != nil { statsConf := stats.Config{} Context.stats.WriteDiskConfig(&statsConf) - config.Stats.Interval = statsConf.LimitDays + config.Stats.Interval = timeutil.Duration{Duration: statsConf.Limit} config.Stats.Enabled = statsConf.Enabled config.Stats.Ignored = statsConf.Ignored.Values() slices.Sort(config.Stats.Ignored) diff --git a/internal/home/dns.go b/internal/home/dns.go index 136e7690..47d6f177 100644 --- a/internal/home/dns.go +++ b/internal/home/dns.go @@ -8,7 +8,6 @@ import ( "net/url" "os" "path/filepath" - "strings" "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" @@ -22,7 +21,6 @@ import ( "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" - "github.com/AdguardTeam/golibs/stringutil" "github.com/ameshkov/dnscrypt/v2" yaml "gopkg.in/yaml.v3" ) @@ -54,13 +52,13 @@ func initDNS() (err error) { statsConf := stats.Config{ Filename: filepath.Join(baseDir, "stats.db"), - LimitDays: config.Stats.Interval, + Limit: config.Stats.Interval.Duration, ConfigModified: onConfigModified, HTTPRegister: httpRegister, Enabled: config.Stats.Enabled, } - set, err := nonDupEmptyHostNames(config.Stats.Ignored) + set, err := aghnet.NewDomainNameSet(config.Stats.Ignored) if err != nil { return fmt.Errorf("statistics: ignored list: %w", err) } @@ -84,13 +82,16 @@ func initDNS() (err error) { FileEnabled: config.QueryLog.FileEnabled, } - set, err = nonDupEmptyHostNames(config.QueryLog.Ignored) + set, err = aghnet.NewDomainNameSet(config.QueryLog.Ignored) if err != nil { return fmt.Errorf("querylog: ignored list: %w", err) } conf.Ignored = set - Context.queryLog = querylog.New(conf) + Context.queryLog, err = querylog.New(conf) + if err != nil { + return fmt.Errorf("init querylog: %w", err) + } Context.filters, err = filtering.New(config.DNS.DnsfilterConf, nil) if err != nil { @@ -535,30 +536,6 @@ func closeDNSServer() { log.Debug("all dns modules are closed") } -// nonDupEmptyHostNames returns nil and error, if list has duplicate or empty -// host name. Otherwise returns a set, which contains lowercase host names -// without dot at the end, and nil error. -func nonDupEmptyHostNames(list []string) (set *stringutil.Set, err error) { - set = stringutil.NewSet() - - for _, v := range list { - host := strings.ToLower(strings.TrimSuffix(v, ".")) - // TODO(a.garipov): Think about ignoring empty (".") names in - // the future. - if host == "" { - return nil, errors.Error("host name is empty") - } - - if set.Has(host) { - return nil, fmt.Errorf("duplicate host name %q", host) - } - - set.Add(host) - } - - return set, nil -} - // safeSearchResolver is a [filtering.Resolver] implementation used for safe // search. type safeSearchResolver struct{} diff --git a/internal/home/upgrade.go b/internal/home/upgrade.go index b2597ad1..79c71673 100644 --- a/internal/home/upgrade.go +++ b/internal/home/upgrade.go @@ -22,7 +22,7 @@ import ( ) // currentSchemaVersion is the current schema version. -const currentSchemaVersion = 19 +const currentSchemaVersion = 20 // These aliases are provided for convenience. type ( @@ -92,6 +92,7 @@ func upgradeConfigSchema(oldVersion int, diskConf yobj) (err error) { upgradeSchema16to17, upgradeSchema17to18, upgradeSchema18to19, + upgradeSchema19to20, } n := 0 @@ -1064,6 +1065,47 @@ func upgradeSchema18to19(diskConf yobj) (err error) { return nil } +// upgradeSchema19to20 performs the following changes: +// +// # BEFORE: +// 'statistics': +// 'interval': 1 +// +// # AFTER: +// 'statistics': +// 'interval': 24h +func upgradeSchema19to20(diskConf yobj) (err error) { + log.Printf("Upgrade yaml: 19 to 20") + diskConf["schema_version"] = 20 + + statsVal, ok := diskConf["statistics"] + if !ok { + return nil + } + + var stats yobj + stats, ok = statsVal.(yobj) + if !ok { + return fmt.Errorf("unexpected type of stats: %T", statsVal) + } + + const field = "interval" + + // Set the initial value from the global configuration structure. + statsIvl := 1 + statsIvlVal, ok := stats[field] + if ok { + statsIvl, ok = statsIvlVal.(int) + if !ok { + return fmt.Errorf("unexpected type of %s: %T", field, statsIvlVal) + } + } + + stats[field] = timeutil.Duration{Duration: time.Duration(statsIvl) * timeutil.Day} + + return nil +} + // TODO(a.garipov): Replace with log.Output when we port it to our logging // package. func funcName() string { diff --git a/internal/home/upgrade_test.go b/internal/home/upgrade_test.go index a713ccf4..193451c0 100644 --- a/internal/home/upgrade_test.go +++ b/internal/home/upgrade_test.go @@ -951,3 +951,98 @@ func TestUpgradeSchema18to19(t *testing.T) { }) } } + +func TestUpgradeSchema19to20(t *testing.T) { + testCases := []struct { + ivl any + want any + wantErr string + name string + }{{ + ivl: 1, + want: timeutil.Duration{Duration: timeutil.Day}, + wantErr: "", + name: "success", + }, { + ivl: 0.25, + want: 0, + wantErr: "unexpected type of interval: float64", + name: "fail", + }} + + for _, tc := range testCases { + conf := yobj{ + "statistics": yobj{ + "interval": tc.ivl, + }, + "schema_version": 19, + } + t.Run(tc.name, func(t *testing.T) { + err := upgradeSchema19to20(conf) + + if tc.wantErr != "" { + require.Error(t, err) + + assert.Equal(t, tc.wantErr, err.Error()) + + return + } + + require.NoError(t, err) + require.Equal(t, conf["schema_version"], 20) + + statsVal, ok := conf["statistics"] + require.True(t, ok) + + var stats yobj + stats, ok = statsVal.(yobj) + require.True(t, ok) + + var newIvl timeutil.Duration + newIvl, ok = stats["interval"].(timeutil.Duration) + require.True(t, ok) + + assert.Equal(t, tc.want, newIvl) + }) + } + + t.Run("no_stats", func(t *testing.T) { + err := upgradeSchema19to20(yobj{}) + + assert.NoError(t, err) + }) + + t.Run("bad_stats", func(t *testing.T) { + err := upgradeSchema19to20(yobj{ + "statistics": 0, + }) + + testutil.AssertErrorMsg(t, "unexpected type of stats: int", err) + }) + + t.Run("no_field", func(t *testing.T) { + conf := yobj{ + "statistics": yobj{}, + } + + err := upgradeSchema19to20(conf) + require.NoError(t, err) + + statsVal, ok := conf["statistics"] + require.True(t, ok) + + var stats yobj + stats, ok = statsVal.(yobj) + require.True(t, ok) + + var ivl any + ivl, ok = stats["interval"] + require.True(t, ok) + + var ivlVal timeutil.Duration + ivlVal, ok = ivl.(timeutil.Duration) + require.True(t, ok) + + assert.Equal(t, 24*time.Hour, ivlVal.Duration) + }) +} diff --git a/internal/querylog/http.go b/internal/querylog/http.go index 1d983d35..d67c962c 100644 --- a/internal/querylog/http.go +++ b/internal/querylog/http.go @@ -13,9 +13,11 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" + "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/stringutil" "github.com/AdguardTeam/golibs/timeutil" + "golang.org/x/exp/slices" "golang.org/x/net/idna" ) @@ -25,8 +27,8 @@ type configJSON struct { // fractional numbers and not mess the API users by changing the units. Interval float64 `json:"interval"` - // Enabled shows if the querylog is enabled. It is an [aghalg.NullBool] - // to be able to tell when it's set without using pointers. + // Enabled shows if the querylog is enabled. It is an aghalg.NullBool to + // be able to tell when it's set without using pointers. Enabled aghalg.NullBool `json:"enabled"` // AnonymizeClientIP shows if the clients' IP addresses must be anonymized. @@ -35,12 +37,39 @@ type configJSON struct { AnonymizeClientIP aghalg.NullBool `json:"anonymize_client_ip"` } +// getConfigResp is the JSON structure for the querylog configuration. +type getConfigResp struct { + // Ignored is the list of host names, which should not be written to log. + Ignored []string `json:"ignored"` + + // Interval is the querylog rotation interval in milliseconds. + Interval float64 `json:"interval"` + + // Enabled shows if the querylog is enabled. It is an aghalg.NullBool to + // be able to tell when it's set without using pointers. + Enabled aghalg.NullBool `json:"enabled"` + + // AnonymizeClientIP shows if the clients' IP addresses must be anonymized. + // It is an aghalg.NullBool to be able to tell when it's set without using + // pointers. + // + // TODO(a.garipov): Consider using separate setting for statistics. + AnonymizeClientIP aghalg.NullBool `json:"anonymize_client_ip"` +} + // Register web handlers func (l *queryLog) initWeb() { l.conf.HTTPRegister(http.MethodGet, "/control/querylog", l.handleQueryLog) l.conf.HTTPRegister(http.MethodGet, "/control/querylog_info", l.handleQueryLogInfo) l.conf.HTTPRegister(http.MethodPost, "/control/querylog_clear", l.handleQueryLogClear) l.conf.HTTPRegister(http.MethodPost, "/control/querylog_config", l.handleQueryLogConfig) + + l.conf.HTTPRegister(http.MethodGet, "/control/querylog/config", l.handleGetQueryLogConfig) + l.conf.HTTPRegister( + http.MethodPut, + "/control/querylog/config/update", + l.handlePutQueryLogConfig, + ) } func (l *queryLog) handleQueryLog(w http.ResponseWriter, r *http.Request) { @@ -64,11 +93,41 @@ func (l *queryLog) handleQueryLogClear(_ http.ResponseWriter, _ *http.Request) { l.clear() } -// Get configuration +// handleQueryLogInfo handles requests to the GET /control/querylog_info +// endpoint. +// +// Deprecated: Remove it when migration to the new API is over. func (l *queryLog) handleQueryLogInfo(w http.ResponseWriter, r *http.Request) { + l.lock.Lock() + defer l.lock.Unlock() + + ivl := l.conf.RotationIvl + + if !checkInterval(ivl) { + // NOTE: If interval is custom we set it to 90 days for compatibility + // with old API. + ivl = timeutil.Day * 90 + } + _ = aghhttp.WriteJSONResponse(w, r, configJSON{ Enabled: aghalg.BoolToNullBool(l.conf.Enabled), - Interval: l.conf.RotationIvl.Hours() / 24, + Interval: ivl.Hours() / 24, + AnonymizeClientIP: aghalg.BoolToNullBool(l.conf.AnonymizeClientIP), + }) +} + +// handleGetQueryLogConfig handles requests to the GET /control/querylog/config +// endpoint. +func (l *queryLog) handleGetQueryLogConfig(w http.ResponseWriter, r *http.Request) { + l.lock.Lock() + defer l.lock.Unlock() + + ignored := l.conf.Ignored.Values() + slices.Sort(ignored) + _ = aghhttp.WriteJSONResponse(w, r, getConfigResp{ + Ignored: ignored, + Interval: float64(l.conf.RotationIvl.Milliseconds()), + Enabled: aghalg.BoolToNullBool(l.conf.Enabled), AnonymizeClientIP: aghalg.BoolToNullBool(l.conf.AnonymizeClientIP), }) } @@ -88,6 +147,8 @@ func AnonymizeIP(ip net.IP) { } // handleQueryLogConfig handles the POST /control/querylog_config queries. +// +// Deprecated: Remove it when migration to the new API is over. func (l *queryLog) handleQueryLogConfig(w http.ResponseWriter, r *http.Request) { // Set NaN as initial value to be able to know if it changed later by // comparing it to NaN. @@ -103,6 +164,7 @@ func (l *queryLog) handleQueryLogConfig(w http.ResponseWriter, r *http.Request) } ivl := time.Duration(float64(timeutil.Day) * newConf.Interval) + hasIvl := !math.IsNaN(newConf.Interval) if hasIvl && !checkInterval(ivl) { aghhttp.Error(r, w, http.StatusBadRequest, "unsupported interval") @@ -115,8 +177,6 @@ func (l *queryLog) handleQueryLogConfig(w http.ResponseWriter, r *http.Request) l.lock.Lock() defer l.lock.Unlock() - // Copy data, modify it, then activate. Other threads (readers) don't need - // to use this lock. conf := *l.conf if newConf.Enabled != aghalg.NBNull { conf.Enabled = newConf.Enabled == aghalg.NBTrue @@ -138,6 +198,65 @@ func (l *queryLog) handleQueryLogConfig(w http.ResponseWriter, r *http.Request) l.conf = &conf } +// handlePutQueryLogConfig handles the PUT /control/querylog/config/update +// queries. +func (l *queryLog) handlePutQueryLogConfig(w http.ResponseWriter, r *http.Request) { + newConf := &getConfigResp{} + err := json.NewDecoder(r.Body).Decode(newConf) + if err != nil { + aghhttp.Error(r, w, http.StatusBadRequest, "%s", err) + + return + } + + set, err := aghnet.NewDomainNameSet(newConf.Ignored) + if err != nil { + aghhttp.Error(r, w, http.StatusUnprocessableEntity, "ignored: %s", err) + + return + } + + ivl := time.Duration(newConf.Interval) * time.Millisecond + err = validateIvl(ivl) + if err != nil { + aghhttp.Error(r, w, http.StatusUnprocessableEntity, "unsupported interval: %s", err) + + return + } + + if newConf.Enabled == aghalg.NBNull { + aghhttp.Error(r, w, http.StatusUnprocessableEntity, "enabled is null") + + return + } + + if newConf.AnonymizeClientIP == aghalg.NBNull { + aghhttp.Error(r, w, http.StatusUnprocessableEntity, "anonymize_client_ip is null") + + return + } + + defer l.conf.ConfigModified() + + l.lock.Lock() + defer l.lock.Unlock() + + conf := *l.conf + + conf.Ignored = set + conf.RotationIvl = ivl + conf.Enabled = newConf.Enabled == aghalg.NBTrue + + conf.AnonymizeClientIP = newConf.AnonymizeClientIP == aghalg.NBTrue + if conf.AnonymizeClientIP { + l.anonymizer.Store(AnonymizeIP) + } else { + l.anonymizer.Store(nil) + } + + l.conf = &conf +} + // "value" -> value, return TRUE func getDoubleQuotesEnclosedValue(s *string) bool { t := *s diff --git a/internal/querylog/qlog.go b/internal/querylog/qlog.go index 3b6c9efe..c46200c5 100644 --- a/internal/querylog/qlog.go +++ b/internal/querylog/qlog.go @@ -132,6 +132,20 @@ func checkInterval(ivl time.Duration) (ok bool) { return ivl == quarterDay || ivl == day || ivl == week || ivl == month || ivl == threeMonths } +// validateIvl returns an error if ivl is less than an hour or more than a +// year. +func validateIvl(ivl time.Duration) (err error) { + if ivl < time.Hour { + return errors.Error("less than an hour") + } + + if ivl > timeutil.Day*365 { + return errors.Error("more than a year") + } + + return nil +} + func (l *queryLog) WriteDiskConfig(c *Config) { *c = *l.conf } @@ -258,6 +272,9 @@ func (l *queryLog) Add(params *AddParams) { // ShouldLog returns true if request for the host should be logged. func (l *queryLog) ShouldLog(host string, _, _ uint16) bool { + l.lock.Lock() + defer l.lock.Unlock() + return !l.isIgnored(host) } diff --git a/internal/querylog/qlog_test.go b/internal/querylog/qlog_test.go index cb9ea3f0..d27c452e 100644 --- a/internal/querylog/qlog_test.go +++ b/internal/querylog/qlog_test.go @@ -22,13 +22,14 @@ func TestMain(m *testing.M) { // TestQueryLog tests adding and loading (with filtering) entries from disk and // memory. func TestQueryLog(t *testing.T) { - l := newQueryLog(Config{ + l, err := newQueryLog(Config{ Enabled: true, FileEnabled: true, RotationIvl: timeutil.Day, MemSize: 100, BaseDir: t.TempDir(), }) + require.NoError(t, err) // Add disk entries. addEntry(l, "example.org", net.IPv4(1, 1, 1, 1), net.IPv4(2, 2, 2, 1)) @@ -125,12 +126,13 @@ func TestQueryLog(t *testing.T) { } func TestQueryLogOffsetLimit(t *testing.T) { - l := newQueryLog(Config{ + l, err := newQueryLog(Config{ Enabled: true, RotationIvl: timeutil.Day, MemSize: 100, BaseDir: t.TempDir(), }) + require.NoError(t, err) const ( entNum = 10 @@ -199,13 +201,14 @@ func TestQueryLogOffsetLimit(t *testing.T) { } func TestQueryLogMaxFileScanEntries(t *testing.T) { - l := newQueryLog(Config{ + l, err := newQueryLog(Config{ Enabled: true, FileEnabled: true, RotationIvl: timeutil.Day, MemSize: 100, BaseDir: t.TempDir(), }) + require.NoError(t, err) const entNum = 10 // Add entries to the log. @@ -227,13 +230,14 @@ func TestQueryLogMaxFileScanEntries(t *testing.T) { } func TestQueryLogFileDisabled(t *testing.T) { - l := newQueryLog(Config{ + l, err := newQueryLog(Config{ Enabled: true, FileEnabled: false, RotationIvl: timeutil.Day, MemSize: 2, BaseDir: t.TempDir(), }) + require.NoError(t, err) addEntry(l, "example1.org", net.IPv4(1, 1, 1, 1), net.IPv4(2, 2, 2, 1)) addEntry(l, "example2.org", net.IPv4(1, 1, 1, 1), net.IPv4(2, 2, 2, 1)) @@ -254,13 +258,14 @@ func TestQueryLogShouldLog(t *testing.T) { ) set := stringutil.NewSet(ignored1, ignored2) - l := newQueryLog(Config{ + l, err := newQueryLog(Config{ Enabled: true, RotationIvl: timeutil.Day, MemSize: 100, BaseDir: t.TempDir(), Ignored: set, }) + require.NoError(t, err) testCases := []struct { name string diff --git a/internal/querylog/querylog.go b/internal/querylog/querylog.go index 28775774..fb1fb9b0 100644 --- a/internal/querylog/querylog.go +++ b/internal/querylog/querylog.go @@ -1,6 +1,7 @@ package querylog import ( + "fmt" "net" "path/filepath" "time" @@ -9,9 +10,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/golibs/errors" - "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/stringutil" - "github.com/AdguardTeam/golibs/timeutil" "github.com/miekg/dns" ) @@ -135,12 +134,12 @@ func (p *AddParams) validate() (err error) { } // New creates a new instance of the query log. -func New(conf Config) (ql QueryLog) { +func New(conf Config) (ql QueryLog, err error) { return newQueryLog(conf) } // newQueryLog crates a new queryLog. -func newQueryLog(conf Config) (l *queryLog) { +func newQueryLog(conf Config) (l *queryLog, err error) { findClient := conf.FindClient if findClient == nil { findClient = func(_ []string) (_ *Client, _ error) { @@ -158,13 +157,10 @@ func newQueryLog(conf Config) (l *queryLog) { l.conf = &Config{} *l.conf = conf - if !checkInterval(conf.RotationIvl) { - log.Info( - "querylog: warning: unsupported rotation interval %s, setting to 1 day", - conf.RotationIvl, - ) - l.conf.RotationIvl = timeutil.Day + err = validateIvl(conf.RotationIvl) + if err != nil { + return nil, fmt.Errorf("unsupported interval: %w", err) } - return l + return l, nil } diff --git a/internal/querylog/search_test.go b/internal/querylog/search_test.go index dbea24a8..939942ad 100644 --- a/internal/querylog/search_test.go +++ b/internal/querylog/search_test.go @@ -35,7 +35,7 @@ func TestQueryLog_Search_findClient(t *testing.T) { return nil, nil } - l := newQueryLog(Config{ + l, err := newQueryLog(Config{ FindClient: findClient, BaseDir: t.TempDir(), RotationIvl: timeutil.Day, @@ -44,6 +44,7 @@ func TestQueryLog_Search_findClient(t *testing.T) { FileEnabled: true, AnonymizeClientIP: false, }) + require.NoError(t, err) t.Cleanup(l.Close) q := &dns.Msg{ diff --git a/internal/stats/http.go b/internal/stats/http.go index 4b041455..6963a410 100644 --- a/internal/stats/http.go +++ b/internal/stats/http.go @@ -7,8 +7,12 @@ import ( "net/http" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" + "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/golibs/timeutil" + "golang.org/x/exp/slices" ) // topAddrs is an alias for the types of the TopFoo fields of statsResponse. @@ -44,7 +48,7 @@ func (s *StatsCtx) handleStats(w http.ResponseWriter, r *http.Request) { defer s.lock.Unlock() start := time.Now() - resp, ok := s.getData(s.limitHours) + resp, ok := s.getData(uint32(s.limit.Hours())) log.Debug("stats: prepared data in %v", time.Since(start)) if !ok { @@ -63,20 +67,62 @@ type configResp struct { IntervalDays uint32 `json:"interval"` } +// getConfigResp is the response to the GET /control/stats_info. +type getConfigResp struct { + // Ignored is the list of host names, which should not be counted. + Ignored []string `json:"ignored"` + + // Interval is the statistics rotation interval in milliseconds. + Interval float64 `json:"interval"` + + // Enabled shows if statistics are enabled. It is an aghalg.NullBool to be + // able to tell when it's set without using pointers. + Enabled aghalg.NullBool `json:"enabled"` +} + // handleStatsInfo handles requests to the GET /control/stats_info endpoint. +// +// Deprecated: Remove it when migration to the new API is over. func (s *StatsCtx) handleStatsInfo(w http.ResponseWriter, r *http.Request) { s.lock.Lock() defer s.lock.Unlock() - resp := configResp{IntervalDays: s.limitHours / 24} + days := uint32(s.limit / timeutil.Day) + ok := checkInterval(days) + if !ok || (s.enabled && days == 0) { + // NOTE: If interval is custom we set it to 90 days for compatibility + // with old API. + days = 90 + } + + resp := configResp{IntervalDays: days} if !s.enabled { resp.IntervalDays = 0 } _ = aghhttp.WriteJSONResponse(w, r, resp) } +// handleGetStatsConfig handles requests to the GET /control/stats/config +// endpoint. +func (s *StatsCtx) handleGetStatsConfig(w http.ResponseWriter, r *http.Request) { + s.lock.Lock() + defer s.lock.Unlock() + + ignored := s.ignored.Values() + slices.Sort(ignored) + + resp := getConfigResp{ + Ignored: ignored, + Interval: float64(s.limit.Milliseconds()), + Enabled: aghalg.BoolToNullBool(s.enabled), + } + _ = aghhttp.WriteJSONResponse(w, r, resp) +} + // handleStatsConfig handles requests to the POST /control/stats_config // endpoint. +// +// Deprecated: Remove it when migration to the new API is over. func (s *StatsCtx) handleStatsConfig(w http.ResponseWriter, r *http.Request) { reqData := configResp{} err := json.NewDecoder(r.Body).Decode(&reqData) @@ -92,8 +138,55 @@ func (s *StatsCtx) handleStatsConfig(w http.ResponseWriter, r *http.Request) { return } - s.setLimit(int(reqData.IntervalDays)) - s.configModified() + defer s.configModified() + + s.lock.Lock() + defer s.lock.Unlock() + + limit := time.Duration(reqData.IntervalDays) * timeutil.Day + s.setLimit(limit) +} + +// handlePutStatsConfig handles requests to the PUT /control/stats/config/update +// endpoint. +func (s *StatsCtx) handlePutStatsConfig(w http.ResponseWriter, r *http.Request) { + reqData := getConfigResp{} + err := json.NewDecoder(r.Body).Decode(&reqData) + if err != nil { + aghhttp.Error(r, w, http.StatusBadRequest, "json decode: %s", err) + + return + } + + set, err := aghnet.NewDomainNameSet(reqData.Ignored) + if err != nil { + aghhttp.Error(r, w, http.StatusUnprocessableEntity, "ignored: %s", err) + + return + } + + ivl := time.Duration(reqData.Interval) * time.Millisecond + err = validateIvl(ivl) + if err != nil { + aghhttp.Error(r, w, http.StatusUnprocessableEntity, "unsupported interval: %s", err) + + return + } + + if reqData.Enabled == aghalg.NBNull { + aghhttp.Error(r, w, http.StatusUnprocessableEntity, "enabled is null") + + return + } + + defer s.configModified() + + s.lock.Lock() + defer s.lock.Unlock() + + s.ignored = set + s.limit = ivl + s.enabled = reqData.Enabled == aghalg.NBTrue } // handleStatsReset handles requests to the POST /control/stats_reset endpoint. @@ -114,4 +207,7 @@ func (s *StatsCtx) initWeb() { s.httpRegister(http.MethodPost, "/control/stats_reset", s.handleStatsReset) s.httpRegister(http.MethodPost, "/control/stats_config", s.handleStatsConfig) s.httpRegister(http.MethodGet, "/control/stats_info", s.handleStatsInfo) + + s.httpRegister(http.MethodGet, "/control/stats/config", s.handleGetStatsConfig) + s.httpRegister(http.MethodPut, "/control/stats/config/update", s.handlePutStatsConfig) } diff --git a/internal/stats/http_test.go b/internal/stats/http_test.go new file mode 100644 index 00000000..c950105f --- /dev/null +++ b/internal/stats/http_test.go @@ -0,0 +1,152 @@ +package stats + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + "time" + + "github.com/AdguardTeam/AdGuardHome/internal/aghalg" + "github.com/AdguardTeam/golibs/testutil" + "github.com/AdguardTeam/golibs/timeutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHandleStatsConfig(t *testing.T) { + const ( + smallIvl = 1 * time.Minute + minIvl = 1 * time.Hour + maxIvl = 365 * timeutil.Day + ) + + conf := Config{ + Filename: filepath.Join(t.TempDir(), "stats.db"), + Limit: time.Hour * 24, + Enabled: true, + UnitID: func() (id uint32) { return 0 }, + ConfigModified: func() {}, + } + + testCases := []struct { + name string + body getConfigResp + wantCode int + wantErr string + }{{ + name: "set_ivl_1_minIvl", + body: getConfigResp{ + Enabled: aghalg.NBTrue, + Interval: float64(minIvl.Milliseconds()), + Ignored: []string{}, + }, + wantCode: http.StatusOK, + wantErr: "", + }, { + name: "small_interval", + body: getConfigResp{ + Enabled: aghalg.NBTrue, + Interval: float64(smallIvl.Milliseconds()), + Ignored: []string{}, + }, + wantCode: http.StatusUnprocessableEntity, + wantErr: "unsupported interval: less than an hour\n", + }, { + name: "big_interval", + body: getConfigResp{ + Enabled: aghalg.NBTrue, + Interval: float64(maxIvl.Milliseconds() + minIvl.Milliseconds()), + Ignored: []string{}, + }, + wantCode: http.StatusUnprocessableEntity, + wantErr: "unsupported interval: more than a year\n", + }, { + name: "set_ignored_ivl_1_maxIvl", + body: getConfigResp{ + Enabled: aghalg.NBTrue, + Interval: float64(maxIvl.Milliseconds()), + Ignored: []string{ + "ignor.ed", + }, + }, + wantCode: http.StatusOK, + wantErr: "", + }, { + name: "ignored_duplicate", + body: getConfigResp{ + Enabled: aghalg.NBTrue, + Interval: float64(minIvl.Milliseconds()), + Ignored: []string{ + "ignor.ed", + "ignor.ed", + }, + }, + wantCode: http.StatusUnprocessableEntity, + wantErr: "ignored: duplicate host name \"ignor.ed\" at index 1\n", + }, { + name: "ignored_empty", + body: getConfigResp{ + Enabled: aghalg.NBTrue, + Interval: float64(minIvl.Milliseconds()), + Ignored: []string{ + "", + }, + }, + wantCode: http.StatusUnprocessableEntity, + wantErr: "ignored: host name is empty\n", + }, { + name: "enabled_is_null", + body: getConfigResp{ + Enabled: aghalg.NBNull, + Interval: float64(minIvl.Milliseconds()), + Ignored: []string{}, + }, + wantCode: http.StatusUnprocessableEntity, + wantErr: "enabled is null\n", + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s, err := New(conf) + require.NoError(t, err) + + s.Start() + testutil.CleanupAndRequireSuccess(t, s.Close) + + buf, err := json.Marshal(tc.body) + require.NoError(t, err) + + const ( + configGet = "/control/stats/config" + configPut = "/control/stats/config/update" + ) + + req := httptest.NewRequest(http.MethodPut, configPut, bytes.NewReader(buf)) + rw := httptest.NewRecorder() + + s.handlePutStatsConfig(rw, req) + require.Equal(t, tc.wantCode, rw.Code) + + if tc.wantCode != http.StatusOK { + assert.Equal(t, tc.wantErr, rw.Body.String()) + + return + } + + resp := httptest.NewRequest(http.MethodGet, configGet, nil) + rw = httptest.NewRecorder() + + s.handleGetStatsConfig(rw, resp) + require.Equal(t, http.StatusOK, rw.Code) + + ans := getConfigResp{} + err = json.Unmarshal(rw.Body.Bytes(), &ans) + require.NoError(t, err) + + assert.Equal(t, tc.body, ans) + }) + } +} diff --git a/internal/stats/stats.go b/internal/stats/stats.go index 9b53874d..9f03fdee 100644 --- a/internal/stats/stats.go +++ b/internal/stats/stats.go @@ -16,6 +16,7 @@ import ( "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/stringutil" + "github.com/AdguardTeam/golibs/timeutil" "go.etcd.io/bbolt" ) @@ -25,6 +26,20 @@ func checkInterval(days uint32) (ok bool) { return days == 0 || days == 1 || days == 7 || days == 30 || days == 90 } +// validateIvl returns an error if ivl is less than an hour or more than a +// year. +func validateIvl(ivl time.Duration) (err error) { + if ivl < time.Hour { + return errors.Error("less than an hour") + } + + if ivl > timeutil.Day*365 { + return errors.Error("more than a year") + } + + return nil +} + // Config is the configuration structure for the statistics collecting. type Config struct { // UnitID is the function to generate the identifier for current unit. If @@ -42,9 +57,8 @@ type Config struct { // Filename is the name of the database file. Filename string - // LimitDays is the maximum number of days to collect statistics into the - // current unit. - LimitDays uint32 + // Limit is an upper limit for collecting statistics. + Limit time.Duration // Enabled tells if the statistics are enabled. Enabled bool @@ -105,11 +119,8 @@ type StatsCtx struct { // enabled tells if the statistics are enabled. enabled bool - // limitHours is the maximum number of hours to collect statistics into the - // current unit. - // - // TODO(s.chzhen): Rewrite to use time.Duration. - limitHours uint32 + // limit is an upper limit for collecting statistics. + limit time.Duration // ignored is the list of host names, which should not be counted. ignored *stringutil.Set @@ -128,9 +139,14 @@ func New(conf Config) (s *StatsCtx, err error) { httpRegister: conf.HTTPRegister, ignored: conf.Ignored, } - if s.limitHours = conf.LimitDays * 24; !checkInterval(conf.LimitDays) { - s.limitHours = 24 + + err = validateIvl(conf.Limit) + if err != nil { + return nil, fmt.Errorf("unsupported interval: %w", err) } + + s.limit = conf.Limit + if s.unitIDGen = newUnitID; conf.UnitID != nil { s.unitIDGen = conf.UnitID } @@ -150,7 +166,7 @@ func New(conf Config) (s *StatsCtx, err error) { return nil, fmt.Errorf("stats: opening a transaction: %w", err) } - deleted := deleteOldUnits(tx, id-s.limitHours-1) + deleted := deleteOldUnits(tx, id-uint32(s.limit.Hours())-1) udb = loadUnitFromDB(tx, id) err = finishTxn(tx, deleted > 0) @@ -231,7 +247,7 @@ func (s *StatsCtx) Update(e Entry) { s.lock.Lock() defer s.lock.Unlock() - if !s.enabled || s.limitHours == 0 { + if !s.enabled || s.limit == 0 { return } @@ -263,7 +279,7 @@ func (s *StatsCtx) WriteDiskConfig(dc *Config) { s.lock.Lock() defer s.lock.Unlock() - dc.LimitDays = s.limitHours / 24 + dc.Limit = s.limit dc.Enabled = s.enabled dc.Ignored = s.ignored } @@ -273,7 +289,7 @@ func (s *StatsCtx) TopClientsIP(maxCount uint) (ips []netip.Addr) { s.lock.Lock() defer s.lock.Unlock() - limit := s.limitHours + limit := uint32(s.limit.Hours()) if !s.enabled || limit == 0 { return nil } @@ -377,7 +393,7 @@ func (s *StatsCtx) flush() (cont bool, sleepFor time.Duration) { return false, 0 } - limit := s.limitHours + limit := uint32(s.limit.Hours()) if limit == 0 || ptr.id == id { return true, time.Second } @@ -436,14 +452,14 @@ func (s *StatsCtx) periodicFlush() { log.Debug("periodic flushing finished") } -func (s *StatsCtx) setLimit(limitDays int) { - s.lock.Lock() - defer s.lock.Unlock() - - if limitDays != 0 { +// setLimit sets the limit. s.lock is expected to be locked. +// +// TODO(s.chzhen): Remove it when migration to the new API is over. +func (s *StatsCtx) setLimit(limit time.Duration) { + if limit != 0 { s.enabled = true - s.limitHours = uint32(24 * limitDays) - log.Debug("stats: set limit: %d days", limitDays) + s.limit = limit + log.Debug("stats: set limit: %d days", limit/timeutil.Day) return } diff --git a/internal/stats/stats_internal_test.go b/internal/stats/stats_internal_test.go index a6ad5ee2..115805fc 100644 --- a/internal/stats/stats_internal_test.go +++ b/internal/stats/stats_internal_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/AdguardTeam/golibs/testutil" + "github.com/AdguardTeam/golibs/timeutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -35,9 +36,9 @@ func TestStats_races(t *testing.T) { var r uint32 idGen := func() (id uint32) { return atomic.LoadUint32(&r) } conf := Config{ - UnitID: idGen, - Filename: filepath.Join(t.TempDir(), "./stats.db"), - LimitDays: 1, + UnitID: idGen, + Filename: filepath.Join(t.TempDir(), "./stats.db"), + Limit: timeutil.Day, } s, err := New(conf) diff --git a/internal/stats/stats_test.go b/internal/stats/stats_test.go index f795c2da..36f9d102 100644 --- a/internal/stats/stats_test.go +++ b/internal/stats/stats_test.go @@ -13,6 +13,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/stats" "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/testutil" + "github.com/AdguardTeam/golibs/timeutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -51,10 +52,10 @@ func TestStats(t *testing.T) { handlers := map[string]http.Handler{} conf := stats.Config{ - Filename: filepath.Join(t.TempDir(), "stats.db"), - LimitDays: 1, - Enabled: true, - UnitID: constUnitID, + Filename: filepath.Join(t.TempDir(), "stats.db"), + Limit: timeutil.Day, + Enabled: true, + UnitID: constUnitID, HTTPRegister: func(_, url string, handler http.HandlerFunc) { handlers[url] = handler }, @@ -158,7 +159,7 @@ func TestLargeNumbers(t *testing.T) { conf := stats.Config{ Filename: filepath.Join(t.TempDir(), "stats.db"), - LimitDays: 1, + Limit: timeutil.Day, Enabled: true, UnitID: func() (id uint32) { return atomic.LoadUint32(&curHour) }, HTTPRegister: func(_, url string, handler http.HandlerFunc) { handlers[url] = handler }, diff --git a/openapi/CHANGELOG.md b/openapi/CHANGELOG.md index 3b43140e..6822f6e0 100644 --- a/openapi/CHANGELOG.md +++ b/openapi/CHANGELOG.md @@ -18,6 +18,71 @@ +## v0.107.27: API changes + +### Deprecated statistics APIs + +* The `GET /control/stats_info` HTTP API; use the new `GET + /control/stats/config` API instead. + + **NOTE:** If `interval` was configured by editing configuration file or new + HTTP API call `PUT /control/stats/config/update` and it's not equal to + previous allowed enum values then it will be equal to `90` days for + compatibility reasons. + +* The `POST /control/stats_config` HTTP API; use the new `PUT + /control/stats/config/update` API instead. + +### New statistics APIs + +* The new `GET /control/stats/config` HTTP API. + +* The new `PUT /control/stats/config/update` HTTP API allows config updates. + +These `control/stats/config/update` and `control/stats/config` APIs accept and +return a JSON object with the following format: + +```json +{ + "enabled": true, + "interval": 3600, + "ignored": ["example.com"], +} +``` + +### Deprecated query log APIs + +* The `GET /control/querylog_info` HTTP API; use the new `GET + /control/querylog/config` API instead. + + **NOTE:** If `interval` was configured by editing configuration file or new + HTTP API call `PUT /control/querylog/config/update` and it's not equal to + previous allowed enum values then it will be equal to `90` days for + compatibility reasons. + +* The `POST /control/querylog_config` HTTP API; use the new `PUT + /control/querylog/config/update` API instead. + +### New query log APIs + +* The new `GET /control/querylog/config` HTTP API. + +* The new `PUT /control/querylog/config/update` HTTP API allows config updates. + +These `control/querylog/config/update` and `control/querylog/config` APIs +accept and return a JSON object with the following format: + +```json +{ + "enabled": true, + "anonymize_client_ip": false, + "interval": 3600, + "ignored": ["example.com"], +} +``` + + + ## v0.107.23: API changes ### Experimental “beta” APIs removed diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 2ec4d858..db83c65c 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -226,6 +226,14 @@ '$ref': '#/components/schemas/QueryLog' '/querylog_info': 'get': + 'deprecated': true + 'description': | + Deprecated: Use `GET /querylog/config` instead. + + NOTE: If `interval` was configured by editing configuration file or new + HTTP API call `PUT /querylog/config/update` and it's not equal to + previous allowed enum values then it will be equal to `90` days for + compatibility reasons. 'tags': - 'log' 'operationId': 'queryLogInfo' @@ -239,6 +247,9 @@ '$ref': '#/components/schemas/QueryLogConfig' '/querylog_config': 'post': + 'deprecated': true + 'description': > + Deprecated: Use `PUT /querylog/config/update` instead. 'tags': - 'log' 'operationId': 'queryLogConfig' @@ -260,6 +271,34 @@ 'responses': '200': 'description': 'OK.' + '/querylog/config': + 'get': + 'tags': + - 'log' + 'operationId': 'getQueryLogConfig' + 'summary': 'Get query log parameters' + 'responses': + '200': + 'description': 'OK.' + 'content': + 'application/json': + 'schema': + '$ref': '#/components/schemas/GetQueryLogConfigResponse' + '/querylog/config/update': + 'put': + 'tags': + - 'log' + 'operationId': 'putQueryLogConfig' + 'summary': 'Set query log parameters' + 'requestBody': + 'content': + 'application/json': + 'schema': + '$ref': '#/components/schemas/PutQueryLogConfigUpdateRequest' + 'required': true + 'responses': + '200': + 'description': 'OK.' '/stats': 'get': 'tags': @@ -284,6 +323,14 @@ 'description': 'OK.' '/stats_info': 'get': + 'deprecated': true + 'description': | + Deprecated: Use `GET /stats/config` instead. + + NOTE: If `interval` was configured by editing configuration file or new + HTTP API call `PUT /stats/config/update` and it's not equal to + previous allowed enum values then it will be equal to `90` days for + compatibility reasons. 'tags': - 'stats' 'operationId': 'statsInfo' @@ -297,6 +344,9 @@ '$ref': '#/components/schemas/StatsConfig' '/stats_config': 'post': + 'deprecated': true + 'description': > + Deprecated: Use `PUT /stats/config/update` instead. 'tags': - 'stats' 'operationId': 'statsConfig' @@ -309,6 +359,34 @@ 'responses': '200': 'description': 'OK.' + '/stats/config': + 'get': + 'tags': + - 'stats' + 'operationId': 'getStatsConfig' + 'summary': 'Get statistics parameters' + 'responses': + '200': + 'description': 'OK.' + 'content': + 'application/json': + 'schema': + '$ref': '#/components/schemas/GetStatsConfigResponse' + '/stats/config/update': + 'put': + 'tags': + - 'stats' + 'operationId': 'putStatsConfig' + 'summary': 'Set statistics parameters' + 'requestBody': + 'content': + 'application/json': + 'schema': + '$ref': '#/components/schemas/PutStatsConfigUpdateRequest' + 'required': true + 'responses': + '200': + 'description': 'OK.' '/tls/status': 'get': 'tags': @@ -1656,6 +1734,27 @@ - 30 - 90 'type': 'integer' + 'GetStatsConfigResponse': + 'type': 'object' + 'description': 'Statistics configuration' + 'required': + - 'enabled' + - 'interval' + - 'ignored' + 'properties': + 'enabled': + 'description': 'Are statistics enabled' + 'type': 'boolean' + 'interval': + 'description': 'Statistics rotation interval' + 'type': 'number' + 'ignored': + 'description': 'List of host names, which should not be counted' + 'type': 'array' + 'items': + 'type': 'string' + 'PutStatsConfigUpdateRequest': + '$ref': '#/components/schemas/GetStatsConfigResponse' 'DhcpConfig': 'type': 'object' 'properties': @@ -2059,6 +2158,32 @@ 'anonymize_client_ip': 'type': 'boolean' 'description': "Anonymize clients' IP addresses" + 'GetQueryLogConfigResponse': + 'type': 'object' + 'description': 'Query log configuration' + 'required': + - 'enabled' + - 'interval' + - 'anonymize_client_ip' + - 'ignored' + 'properties': + 'enabled': + 'type': 'boolean' + 'description': 'Is query log enabled' + 'interval': + 'description': > + Time period for query log rotation. + 'type': 'number' + 'anonymize_client_ip': + 'type': 'boolean' + 'description': "Anonymize clients' IP addresses" + 'ignored': + 'description': 'List of host names, which should not be written to log' + 'type': 'array' + 'items': + 'type': 'string' + 'PutQueryLogConfigUpdateRequest': + '$ref': '#/components/schemas/GetQueryLogConfigResponse' 'ResultRule': 'description': 'Applied rule.' 'properties':