diff --git a/.rstcheck.cfg b/.rstcheck.cfg new file mode 100644 index 0000000000000..b19ce17f48dae --- /dev/null +++ b/.rstcheck.cfg @@ -0,0 +1,5 @@ +[rstcheck] +ignore_directives=ifconfig,substitution-code-block,tabs,validated-code-block +ignore_roles=repo +# reenable this +ignore_messages=(Hyperlink) diff --git a/BUILD b/BUILD index 7ef982f5621bc..6f8961a776db9 100644 --- a/BUILD +++ b/BUILD @@ -6,6 +6,7 @@ exports_files([ ".clang-format", "pytest.ini", ".coveragerc", + ".rstcheck.cfg", ]) # These two definitions exist to help reduce Envoy upstream core code depending on extensions. diff --git a/bazel/repositories_extra.bzl b/bazel/repositories_extra.bzl index bda3919952dd9..a7f4deb786495 100644 --- a/bazel/repositories_extra.bzl +++ b/bazel/repositories_extra.bzl @@ -41,6 +41,11 @@ def _python_deps(): requirements = "@envoy//tools/dependency:requirements.txt", extra_pip_args = ["--require-hashes"], ) + pip_install( + name = "docs_pip3", + requirements = "@envoy//docs:requirements.txt", + extra_pip_args = ["--require-hashes"], + ) pip_install( name = "kafka_pip3", requirements = "@envoy//source/extensions/filters/network/kafka:requirements.txt", diff --git a/ci/do_ci.sh b/ci/do_ci.sh index f906a04bbd7bb..74ac78efce915 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -474,6 +474,7 @@ elif [[ "$CI_TARGET" == "tooling" ]]; then bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/base:pytest_runner -- --cov-collect /tmp/.coverage-envoy bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/base:pytest_utils -- --cov-collect /tmp/.coverage-envoy bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/code_format:pytest_python_check -- --cov-collect /tmp/.coverage-envoy + bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/code_format:pytest_rst_check -- --cov-collect /tmp/.coverage-envoy bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/testing:python_coverage -- --fail-under=95 /tmp/.coverage-envoy /source/generated/tooling exit 0 elif [[ "$CI_TARGET" == "verify_examples" ]]; then diff --git a/docs/build.sh b/docs/build.sh index 7b6eee6ba76d4..9c59bdb0fd058 100755 --- a/docs/build.sh +++ b/docs/build.sh @@ -171,7 +171,17 @@ rsync -av \ "${SCRIPT_DIR}"/_ext \ "${GENERATED_RST_DIR}" +LINTFAIL= + +bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/code_format:rst_check "${GENERATED_RST_DIR}" || { + LINTFAIL=yes +} + # To speed up validate_fragment invocations in validating_code_block bazel build "${BAZEL_BUILD_OPTIONS[@]}" //tools/config_validation:validate_fragment sphinx-build -W --keep-going -b html "${GENERATED_RST_DIR}" "${DOCS_OUTPUT_DIR}" + +if [[ -n "$LINTFAIL" ]]; then + exit 1 +fi diff --git a/docs/requirements.txt b/docs/requirements.txt index 8e2a18417d71d..183ad7ddd7351 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -77,6 +77,11 @@ pytz==2021.1 \ requests==2.25.1 \ --hash=sha256:c210084e36a42ae6b9219e00e48287def368a26d03a048ddad7bfee44f75871e \ --hash=sha256:27973dd4a904a4f13b263a19c866c13b92a39ed1c964655f025f3f8d3d75b804 +rstcheck==3.3.1 \ + --hash=sha256:92c4f79256a54270e0402ba16a2f92d0b3c15c8f4410cb9c57127067c215741f +setuptools==54.2.0 \ + --hash=sha256:b726461910b9ba30f077880c228bea22121aec50b172edf39eb7ff026c054a11 \ + --hash=sha256:aa9c24fb83a9116b8d425e53bec24c7bfdbffc313c2159f9ed036d4a6dd32d7d six==1.15.0 \ --hash=sha256:30639c035cdb23534cd4aa2dd52c3bf48f06e5f4a941509c8bafd8ce11080259 \ --hash=sha256:8b74bedcbbbaca38ff6d7491d76f2b06b3592611af620f8426e82dddb04a5ced diff --git a/docs/root/api/api.rst b/docs/root/api/api.rst index 7d72c38ca37e9..d612c2e89b061 100644 --- a/docs/root/api/api.rst +++ b/docs/root/api/api.rst @@ -1,4 +1,3 @@ -.. _api: API === diff --git a/docs/root/configuration/http/http_conn_man/headers.rst b/docs/root/configuration/http/http_conn_man/headers.rst index d3a9d2aa96d62..b60d7c17f0aab 100644 --- a/docs/root/configuration/http/http_conn_man/headers.rst +++ b/docs/root/configuration/http/http_conn_man/headers.rst @@ -304,12 +304,11 @@ Example 6: The internal Envoy from Example 5, receiving a request proxied by ano A few very important notes about XFF: +.. _config_http_conn_man_headers_x-forwarded-for_internal_origin: + 1. If *use_remote_address* is set to true, Envoy sets the :ref:`config_http_conn_man_headers_x-envoy-external-address` header to the trusted client address. - -.. _config_http_conn_man_headers_x-forwarded-for_internal_origin: - 2. XFF is what Envoy uses to determine whether a request is internal origin or external origin. If *use_remote_address* is set to true, the request is internal if and only if the request contains no XFF and the immediate downstream node's connection to Envoy has diff --git a/docs/root/intro/arch_overview/other_features/compression/libraries.rst b/docs/root/intro/arch_overview/other_features/compression/libraries.rst index c6dbe69991aa0..aac28e3dfa1b2 100644 --- a/docs/root/intro/arch_overview/other_features/compression/libraries.rst +++ b/docs/root/intro/arch_overview/other_features/compression/libraries.rst @@ -6,15 +6,15 @@ Compression Libraries Underlying implementation ------------------------- -Currently Envoy uses `zlib `_ and `brotli `_ as compression +Currently Envoy uses `zlib `__ and `brotli `_ as compression libraries. .. note:: - `zlib-ng `_ is a fork that hosts several 3rd-party + `zlib-ng `__ is a fork that hosts several 3rd-party contributions containing new optimizations. Those optimizations are considered useful for `improving compression performance `_. - Envoy can be built to use `zlib-ng `_ instead of regular - `zlib `_ by using ``--define zlib=ng`` Bazel option. The relevant build options - used to build `zlib-ng `_ can be evaluated in :repo:`here + Envoy can be built to use `zlib-ng `__ instead of regular + `zlib `__ by using ``--define zlib=ng`` Bazel option. The relevant build options + used to build `zlib-ng `__ can be evaluated in :repo:`here `. Currently, this option is only available on Linux. diff --git a/docs/root/intro/arch_overview/other_protocols/grpc.rst b/docs/root/intro/arch_overview/other_protocols/grpc.rst index 01275183b1553..e01a8c31eab62 100644 --- a/docs/root/intro/arch_overview/other_protocols/grpc.rst +++ b/docs/root/intro/arch_overview/other_protocols/grpc.rst @@ -3,7 +3,7 @@ gRPC ==== -`gRPC `_ is an RPC framework from Google. It uses protocol buffers as the +`gRPC `__ is an RPC framework from Google. It uses protocol buffers as the underlying serialization/IDL format. At the transport layer it uses HTTP/2 for request/response multiplexing. Envoy has first class support for gRPC both at the transport layer as well as at the application layer: diff --git a/docs/root/intro/arch_overview/other_protocols/redis.rst b/docs/root/intro/arch_overview/other_protocols/redis.rst index d8da73a0f2b9f..7cb1e75b063be 100644 --- a/docs/root/intro/arch_overview/other_protocols/redis.rst +++ b/docs/root/intro/arch_overview/other_protocols/redis.rst @@ -6,7 +6,7 @@ Redis Envoy can act as a Redis proxy, partitioning commands among instances in a cluster. In this mode, the goals of Envoy are to maintain availability and partition tolerance over consistency. This is the key point when comparing Envoy to `Redis Cluster -`_. Envoy is designed as a best-effort cache, +`__. Envoy is designed as a best-effort cache, meaning that it will not try to reconcile inconsistent data or keep a globally consistent view of cluster membership. It also supports routing commands from different workload to different to different upstream clusters based on their access patterns, eviction, or isolation @@ -63,9 +63,9 @@ close map to 5xx. All other responses from Redis are counted as a success. .. _arch_overview_redis_cluster_support: Redis Cluster Support (Experimental) ----------------------------------------- +------------------------------------ -Envoy currently offers experimental support for `Redis Cluster `_. +Envoy currently offers experimental support for `Redis Cluster `__. When using Envoy as a sidecar proxy for a Redis Cluster, the service can use a non-cluster Redis client implemented in any language to connect to the proxy as if it's a single node Redis instance. diff --git a/docs/root/intro/arch_overview/security/ssl.rst b/docs/root/intro/arch_overview/security/ssl.rst index 9849fda2df9d2..be58b07b61641 100644 --- a/docs/root/intro/arch_overview/security/ssl.rst +++ b/docs/root/intro/arch_overview/security/ssl.rst @@ -43,7 +43,7 @@ FIPS 140-2 BoringSSL can be built in a `FIPS-compliant mode `_, following the build instructions from the `Security Policy for BoringCrypto module -`_, +`__, using ``--define boringssl=fips`` Bazel option. Currently, this option is only available on Linux-x86_64. The correctness of the FIPS build can be verified by checking the presence of ``BoringSSL-FIPS`` @@ -54,7 +54,7 @@ it's not sufficient by itself, and depending on the context, additional steps mi The extra requirements may include using only approved algorithms and/or using only private keys generated by a module operating in FIPS-approved mode. For more information, please refer to the `Security Policy for BoringCrypto module -`_ +`__ and/or an `accredited CMVP laboratory `_. Please note that the FIPS-compliant build is based on an older version of BoringSSL than diff --git a/docs/root/start/building.rst b/docs/root/start/building.rst index 297c403971d08..745649340d973 100644 --- a/docs/root/start/building.rst +++ b/docs/root/start/building.rst @@ -1,5 +1,3 @@ -.. _building: - Building ======== diff --git a/tools/base/checker.py b/tools/base/checker.py index 4bdef7a5632bd..c1bc02caf60b4 100644 --- a/tools/base/checker.py +++ b/tools/base/checker.py @@ -58,7 +58,7 @@ def path(self) -> str: raise self.parser.error( "Incorrect path: `path` must be a directory, set either as first arg or with --path" ) - return path + return path.rstrip("/") @property def paths(self) -> list: diff --git a/tools/base/runner.py b/tools/base/runner.py index be1bf1e8bec7c..d7d8a361007c9 100644 --- a/tools/base/runner.py +++ b/tools/base/runner.py @@ -4,6 +4,7 @@ import argparse import logging +import multiprocessing import os import subprocess import sys @@ -73,6 +74,15 @@ def add_arguments(self, parser: argparse.ArgumentParser) -> None: """Override this method to add custom arguments to the arg parser""" pass + def parallel(self, cmd, args, handle): + errors = 0 + pool = multiprocessing.Pool(multiprocessing.cpu_count()) + try: + handle(pool.map(cmd, args)) + finally: + pool.close() + return errors + class ForkingAdapter(object): diff --git a/tools/code_format/BUILD b/tools/code_format/BUILD index 95784d0ef412f..246c875703c26 100644 --- a/tools/code_format/BUILD +++ b/tools/code_format/BUILD @@ -1,4 +1,5 @@ load("@pylint_pip3//:requirements.bzl", "requirement") +load("@docs_pip3//:requirements.bzl", doc_requirement = "requirement") load("//bazel:envoy_build_system.bzl", "envoy_package") load("//tools/base:envoy_python.bzl", "envoy_py_binary") @@ -22,3 +23,16 @@ envoy_py_binary( requirement("yapf"), ], ) + +envoy_py_binary( + name = "tools.code_format.rst_check", + data = [ + "//:.rstcheck.cfg", + ], + deps = [ + "//tools/base:checker", + "//tools/base:utils", + doc_requirement("rstcheck"), + doc_requirement("Sphinx"), + ], +) diff --git a/tools/code_format/rst_check.py b/tools/code_format/rst_check.py new file mode 100755 index 0000000000000..1cfccb4e0f0c8 --- /dev/null +++ b/tools/code_format/rst_check.py @@ -0,0 +1,161 @@ +#!/usr/bin/python3 + +import argparse +import configparser +import multiprocessing +import re +import os +import shutil +import subprocess +import sys +import tempfile +from contextlib import closing, contextmanager +from functools import cached_property + +import docutils +import rstcheck +import sphinx + +from tools.base import checker, utils + + +# things we dont want to see in generated docs +# TODO(phlax): move to .rstcheck.cfg when available +RSTCHECK_GREP_FAIL = (" ref:", "\\[\\#") +RSTCHECK_CONFIG=".rstcheck.cfg" +RSTCHECK_COMMAND = ("rstcheck", "-r") + + +@contextmanager +def sphinx_enabled(): + """Register Sphinx directives and roles.""" + srcdir = tempfile.mkdtemp() + outdir = os.path.join(srcdir, '_build') + try: + sphinx.application.Sphinx( + srcdir=srcdir, + confdir=None, + outdir=outdir, + doctreedir=outdir, + buildername='dummy', + status=None) + yield + finally: + shutil.rmtree(srcdir) + + +def _check_rst_file(params: tuple) -> list: + """Check an rst file and return list of errors. + + This function *must* be module level in order to be pickled + for multiprocessing + """ + filename, args = params + + with closing(docutils.io.FileInput(source_path=filename)) as input_file: + contents = input_file.read() + + rstcheck.ignore_directives_and_roles( + args.ignore_directives, args.ignore_roles) + + for substitution in args.ignore_substitutions: + contents = contents.replace(f"|{substitution}|", "None") + + ignore = { + "languages": args.ignore_language, + "messages": args.ignore_messages} + return filename, list( + rstcheck.check( + contents, + filename=filename, + report_level=args.report, + ignore=ignore)) + + +class RstChecker(checker.ForkingChecker): + checks = ("greps", "rstcheck") + + @property + def failing_regexes(self) -> tuple: + return RSTCHECK_GREP_FAIL + + @cached_property + def rstcheck_args(self) -> argparse.Namespace: + """Return new ``args`` with configuration loaded from file.""" + args = argparse.Namespace() + args.report = self.rstcheck_report_level + for ignore in ["language", "directives", "substitutions", "roles"]: + setattr( + args, + f"ignore_{ignore}", + [o.strip() + for o in self.rstcheck_config.get(f"ignore_{ignore}", "").split(",") + if o.strip()]) + args.ignore_messages = self.rstcheck_config.get("ignore_messages", "") + return args + + @cached_property + def rstcheck_config(self) -> dict: + parser = configparser.ConfigParser() + parser.read(rstcheck.find_config(self.path)) + try: + return dict(parser.items("rstcheck")) + except configparser.NoSectionError: + return {} + + @cached_property + def rstcheck_report_level(self) -> int: + report = self.rstcheck_config.get("report", "info") + threshold_dictionary = docutils.frontend.OptionParser.thresholds + return int(threshold_dictionary.get(report, report)) + + @cached_property + def rst_files(self) -> list: + return list( + rstcheck.find_files( + filenames=self.paths, + recursive=True)) + + def check_greps(self) -> None: + for check in self.failing_regexes: + self.grep_unwanted(check) + + def check_rstcheck(self) -> None: + with sphinx_enabled(): + try: + self.parallel( + _check_rst_file, + [(path, self.rstcheck_args) for path in self.rst_files], + self._rstcheck_handle) + except (IOError, UnicodeError) as e: + self.error("rstcheck", [f"Rstcheck failed: {e}"]) + + def grep_unwanted(self, check: str) -> None: + response = self.fork(["grep", "-nr", "--include", "\\*.rst"] + [check]) + if not response.returncode: + self.error("grep", [f"grepping found bad '{check}': {path}" for path in resp.stdout.split("\n")]) + else: + self.succeed("grep", [f"grepping found no errors for '{check}'"]) + + def _rstcheck_handle(self, results: list) -> None: + """Handle multiprocessed error results of rstchecks""" + for (filename, _errors) in results: + for (line_number, message) in _errors: + if not re.match(r'\([A-Z]+/[0-9]+\)', message): + message = '(ERROR/3) ' + message + self.error( + "rstcheck", + [f"{self._strip_line(filename)}:{line_number}: {message}"]) + else: + self.succeed("rstcheck", [f"No errors for: {filename}"]) + + def _strip_line(self, line: str) -> str: + return line[len(self.path) + 1:] if line.startswith(f"{self.path}/") else line + + +def main(*args: list) -> None: + return RstChecker(*args).run_checks() + + +if __name__ == "__main__": + sys.exit(main(*sys.argv[1:])) diff --git a/tools/code_format/tests/test_rst_check.py b/tools/code_format/tests/test_rst_check.py new file mode 100644 index 0000000000000..92c3724c88d9e --- /dev/null +++ b/tools/code_format/tests/test_rst_check.py @@ -0,0 +1,20 @@ + +from unittest.mock import patch + +from tools.code_format import rst_check + + +def test_python_checker_main(): + class_mock = patch("tools.code_format.rst_check.RstChecker") + + with class_mock as m_class: + assert ( + rst_check.main("arg0", "arg1", "arg2") + == m_class.return_value.run_checks.return_value) + + assert ( + list(m_class.call_args) + == [('arg0', 'arg1', 'arg2'), {}]) + assert ( + list(m_class.return_value.run_checks.call_args) + == [(), {}])