diff --git a/.github/workflows/_cache.yml b/.github/workflows/_cache.yml index a30cf01df7..e86e3ddb44 100644 --- a/.github/workflows/_cache.yml +++ b/.github/workflows/_cache.yml @@ -48,13 +48,13 @@ jobs: docker: runs-on: ${{ inputs.runs-on || 'ubuntu-24.04' }} steps: - - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 id: appauth name: Appauth (mutex lock) with: app_id: ${{ secrets.app-id }} key: ${{ secrets.app-key }} - - uses: envoyproxy/toolshed/gh-actions/docker/cache/prime@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/docker/cache/prime@actions-v0.2.36 id: docker name: Prime Docker cache (${{ inputs.image-tag }}${{ inputs.cache-suffix }}) with: @@ -62,7 +62,7 @@ jobs: key-suffix: ${{ inputs.cache-suffix }} lock-token: ${{ steps.appauth.outputs.token }} lock-repository: ${{ inputs.lock-repository }} - - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.36 id: data name: Cache data with: @@ -70,7 +70,7 @@ jobs: input: | cached: ${{ steps.docker.outputs.cached }} key: ${{ inputs.image-tag }}${{ inputs.cache-suffix }} - - uses: envoyproxy/toolshed/gh-actions/json/table@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/json/table@actions-v0.2.36 name: Summary with: json: ${{ steps.data.outputs.value }} diff --git a/.github/workflows/_check_coverage.yml b/.github/workflows/_check_coverage.yml index 561c7759db..3ad92610f9 100644 --- a/.github/workflows/_check_coverage.yml +++ b/.github/workflows/_check_coverage.yml @@ -34,6 +34,7 @@ jobs: # bazel-extra: '--config=remote-envoy-engflow' cache-build-image: ${{ fromJSON(inputs.request).request.build-image.default }} concurrency-suffix: -${{ matrix.target }} + diskspace-hack: ${{ matrix.diskspace-hack || false }} error-match: | ERROR error: @@ -41,6 +42,12 @@ jobs: lower than limit rbe: true request: ${{ inputs.request }} + steps-post: | + - run: ci/run_envoy_docker.sh 'ci/do_ci.sh ${{ matrix.target }}-upload' + shell: bash + env: + GCS_ARTIFACT_BUCKET: ${{ inputs.trusted && 'envoy-postsubmit' || 'envoy-pr' }} + GCS_REDIRECT_PATH: ${{ fromJSON(inputs.request).request.pr || fromJSON(inputs.request).request.target-branch }} target: ${{ matrix.target }} timeout-minutes: 180 trusted: ${{ inputs.trusted }} @@ -50,5 +57,6 @@ jobs: include: - target: coverage name: Coverage + diskspace-hack: true - target: fuzz_coverage name: Fuzz coverage diff --git a/.github/workflows/_finish.yml b/.github/workflows/_finish.yml index 62a7a884b0..e1c2ef4326 100644 --- a/.github/workflows/_finish.yml +++ b/.github/workflows/_finish.yml @@ -36,7 +36,7 @@ jobs: actions: read contents: read steps: - - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.36 name: Incoming data id: needs with: @@ -87,7 +87,7 @@ jobs: summary: "Check has finished", text: $text}}}} - - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.36 name: Print summary with: input: ${{ toJSON(steps.needs.outputs.value).summary-title }} @@ -95,13 +95,13 @@ jobs: "## \(.)" options: -Rr output-path: GITHUB_STEP_SUMMARY - - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 name: Appauth id: appauth with: app_id: ${{ secrets.app-id }} key: ${{ secrets.app-key }} - - uses: envoyproxy/toolshed/gh-actions/github/checks@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/checks@actions-v0.2.36 name: Update check with: action: update diff --git a/.github/workflows/_load.yml b/.github/workflows/_load.yml index 5c06e9c99c..cca4813c6c 100644 --- a/.github/workflows/_load.yml +++ b/.github/workflows/_load.yml @@ -107,7 +107,7 @@ jobs: # Handle any failure in triggering job # Remove any `checks` we dont care about # Prepare a check request - - uses: envoyproxy/toolshed/gh-actions/github/env/load@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/env/load@actions-v0.2.36 name: Load env id: data with: @@ -118,13 +118,13 @@ jobs: GH_TOKEN: ${{ github.token }} # Update the check - - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 name: Appauth id: appauth with: app_id: ${{ secrets.app-id }} key: ${{ secrets.app-key }} - - uses: envoyproxy/toolshed/gh-actions/github/checks@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/checks@actions-v0.2.36 name: Update check if: ${{ fromJSON(steps.data.outputs.data).data.check.action == 'RUN' }} with: @@ -132,7 +132,7 @@ jobs: checks: ${{ toJSON(fromJSON(steps.data.outputs.data).checks) }} token: ${{ steps.appauth.outputs.token }} - - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.36 name: Print request summary with: input: | @@ -152,7 +152,7 @@ jobs: | $summary.summary as $summary | "${{ inputs.template-request-summary }}" - - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.36 id: request-output name: Load request with: diff --git a/.github/workflows/_load_env.yml b/.github/workflows/_load_env.yml index 6c9f4f7d5d..bc95b73246 100644 --- a/.github/workflows/_load_env.yml +++ b/.github/workflows/_load_env.yml @@ -63,18 +63,18 @@ jobs: request: ${{ steps.env.outputs.data }} trusted: true steps: - - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.36 id: started name: Create timestamp with: options: -r filter: | now - - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.36 id: checkout name: Checkout Envoy repository - name: Generate environment variables - uses: envoyproxy/toolshed/gh-actions/envoy/ci/env@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/envoy/ci/env@actions-v0.2.36 id: env with: branch-name: ${{ inputs.branch-name }} @@ -86,7 +86,7 @@ jobs: - name: Request summary id: summary - uses: envoyproxy/toolshed/gh-actions/github/env/summary@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/github/env/summary@actions-v0.2.36 with: actor: ${{ toJSON(fromJSON(steps.env.outputs.data).request.actor) }} base-sha: ${{ fromJSON(steps.env.outputs.data).request.base-sha }} diff --git a/.github/workflows/_precheck_publish.yml b/.github/workflows/_precheck_publish.yml index 86181a7bc3..a8b6ae02a4 100644 --- a/.github/workflows/_precheck_publish.yml +++ b/.github/workflows/_precheck_publish.yml @@ -5,6 +5,9 @@ permissions: on: workflow_call: + secrets: + gcp-key: + required: true inputs: request: type: string @@ -20,6 +23,8 @@ concurrency: jobs: publish: + secrets: + gcp-key: ${{ secrets.gcp-key }} permissions: contents: read packages: read @@ -30,6 +35,7 @@ jobs: cache-build-image: ${{ fromJSON(inputs.request).request.build-image.default }} cache-build-image-key-suffix: ${{ matrix.arch == 'arm64' && '-arm64' || '' }} concurrency-suffix: -${{ matrix.target }}${{ matrix.arch && format('-{0}', matrix.arch) || '' }} + gcs-only: "true" rbe: ${{ matrix.rbe }} request: ${{ inputs.request }} runs-on: ${{ matrix.runs-on || 'ubuntu-24.04' }} @@ -38,7 +44,9 @@ jobs: ERROR error: Error: + steps-post: ${{ matrix.steps-post }} target: ${{ matrix.target }} + target-suffix: ${{ matrix.target-suffix }} trusted: ${{ inputs.trusted }} strategy: fail-fast: false @@ -46,10 +54,12 @@ jobs: include: - target: release.test_only name: Release (x64) + target-suffix: x64 arch: x64 rbe: true - target: release.test_only name: Release (arm64) + target-suffix: arm64 arch: arm64 bazel-extra: >- --config=common-envoy-engflow @@ -64,3 +74,9 @@ jobs: --config=remote-envoy-engflow --config=docs-ci rbe: true + steps-post: | + - run: ci/run_envoy_docker.sh 'ci/do_ci.sh docs-upload' + shell: bash + env: + GCS_ARTIFACT_BUCKET: ${{ inputs.trusted && 'envoy-postsubmit' || 'envoy-pr' }} + GCS_REDIRECT_PATH: ${{ fromJSON(inputs.request).request.pr || fromJSON(inputs.request).request.target-branch }} diff --git a/.github/workflows/_publish_build.yml b/.github/workflows/_publish_build.yml index e89c92c650..f03d887a6f 100644 --- a/.github/workflows/_publish_build.yml +++ b/.github/workflows/_publish_build.yml @@ -41,6 +41,7 @@ jobs: with: bazel-extra: ${{ matrix.bazel-extra }} target: ${{ matrix.target }} + target-suffix: ${{ matrix.arch }} cache-build-image: ${{ fromJSON(inputs.request).request.build-image.default }} cache-build-image-key-suffix: ${{ matrix.arch == 'arm64' && format('-{0}', matrix.arch) || '' }} concurrency-suffix: -${{ matrix.arch }} @@ -91,6 +92,7 @@ jobs: downloads: | release.${{ matrix.arch }}: release/${{ matrix.arch }}/bin/ target: ${{ matrix.target }} + target-suffix: ${{ matrix.arch }} cache-build-image: ${{ fromJSON(inputs.request).request.build-image.default }} cache-build-image-key-suffix: ${{ matrix.cache-build-image-key-suffix }} concurrency-suffix: -${{ matrix.arch }} diff --git a/.github/workflows/_publish_publish.yml b/.github/workflows/_publish_publish.yml index 36bb5f0fd4..169a751646 100644 --- a/.github/workflows/_publish_publish.yml +++ b/.github/workflows/_publish_publish.yml @@ -71,12 +71,12 @@ jobs: needs: - publish steps: - - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 id: appauth with: app_id: ${{ secrets.ENVOY_CI_SYNC_APP_ID }} key: ${{ secrets.ENVOY_CI_SYNC_APP_KEY }} - - uses: envoyproxy/toolshed/gh-actions/dispatch@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/dispatch@actions-v0.2.36 with: ref: main repository: ${{ fromJSON(inputs.request).request.version.dev && 'envoyproxy/envoy-website' || 'envoyproxy/archive' }} diff --git a/.github/workflows/_publish_verify.yml b/.github/workflows/_publish_verify.yml index d4838c3442..2f95ee5504 100644 --- a/.github/workflows/_publish_verify.yml +++ b/.github/workflows/_publish_verify.yml @@ -99,10 +99,11 @@ jobs: export NO_BUILD_SETUP=1 export ENVOY_DOCKER_IN_DOCKER=1 target: ${{ matrix.target }} + target-suffix: ${{ matrix.arch }} trusted: ${{ inputs.trusted }} steps-pre: | - run: | - echo ARCH=${{ matrix.arch || 'x64' }} >> $GITHUB_ENV + echo ARCH=${{ matrix.arch }} >> $GITHUB_ENV echo DEB_ARCH=${{ matrix.arch == 'arm64' && 'arm64' || 'amd64' }} >> $GITHUB_ENV shell: bash - run: | @@ -124,6 +125,7 @@ jobs: - name: verify_distro_x64 target: verify_distro + arch: x64 rbe: true - name: verify_distro_arm64 diff --git a/.github/workflows/_request.yml b/.github/workflows/_request.yml index 1b44ae8101..d13845e636 100644 --- a/.github/workflows/_request.yml +++ b/.github/workflows/_request.yml @@ -40,14 +40,14 @@ jobs: env: ${{ steps.data.outputs.value }} config: ${{ steps.config.outputs.config }} steps: - - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.36 id: started name: Create timestamp with: options: -r filter: | now - - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.36 id: checkout name: Checkout Envoy repository with: @@ -60,7 +60,7 @@ jobs: # *ALL* variables collected should be treated as untrusted and should be sanitized before # use - name: Generate environment variables from commit - uses: envoyproxy/toolshed/gh-actions/envoy/ci/request@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/envoy/ci/request@actions-v0.2.36 id: env with: branch-name: ${{ steps.checkout.outputs.branch-name }} @@ -71,7 +71,7 @@ jobs: vars: ${{ toJSON(vars) }} - name: Request summary id: summary - uses: envoyproxy/toolshed/gh-actions/github/env/summary@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/github/env/summary@actions-v0.2.36 with: actor: ${{ toJSON(fromJSON(steps.env.outputs.data).request.actor) }} base-sha: ${{ fromJSON(steps.env.outputs.data).request.base-sha }} @@ -87,7 +87,7 @@ jobs: target-branch: ${{ fromJSON(steps.env.outputs.data).request.target-branch }} - name: Environment data - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.36 id: data with: input: | diff --git a/.github/workflows/_run.yml b/.github/workflows/_run.yml index 3a907421cd..612b8f2410 100644 --- a/.github/workflows/_run.yml +++ b/.github/workflows/_run.yml @@ -69,6 +69,8 @@ on: Error: fail-match: type: string + gcs-only: + type: string import-gpg: type: boolean default: false @@ -90,15 +92,16 @@ on: type: string default: | - run: | - echo "disk space at beginning of build:" - df -h + # disk space at beginning of build + df -h > "${TMP_REPORT}/df-pre" shell: bash report-post: type: string default: | - run: | - echo "disk space at end of build:" - df -h + # disk space at end of build + df -h > "${TMP_REPORT}/df-post" + (du -ch "%{{ inputs.temp-dir || runner.temp }}" | grep -E "[0-9]{2,}M|[0-9]G" || :) > "${TMP_REPORT}/du-post" shell: bash request: type: string @@ -123,15 +126,15 @@ on: type: string steps-post: type: string - default: | - - run: | - du -ch "%{{ inputs.temp-dir || runner.temp }}" | grep -E "[0-9]{2,}M|[0-9]G" || : - shell: bash steps-post-name: type: string target: type: string required: true + target-name: + type: string + target-suffix: + type: string temp-dir: type: string timeout-minutes: @@ -173,10 +176,10 @@ jobs: packages: read if: ${{ ! inputs.skip }} runs-on: ${{ inputs.runs-on || fromJSON(inputs.request).config.ci.agent-ubuntu }} - name: ${{ inputs.command }} ${{ inputs.target }} + name: ${{ inputs.target-suffix && format('[{0}] ', inputs.target-suffix) || '' }}${{ inputs.command }} ${{ inputs.target }} timeout-minutes: ${{ inputs.timeout-minutes }} steps: - - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.36 id: started name: Create timestamp with: @@ -184,7 +187,7 @@ jobs: filter: | now # This controls which input vars are exposed to the run action (and related steps) - - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.36 name: Context id: context with: @@ -205,12 +208,12 @@ jobs: | . * {$config, $check} - if: ${{ inputs.cache-build-image }} name: Restore Docker cache ${{ inputs.cache-build-image && format('({0})', inputs.cache-build-image) || '' }} - uses: envoyproxy/toolshed/gh-actions/docker/cache/restore@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/docker/cache/restore@actions-v0.2.36 with: image_tag: ${{ inputs.cache-build-image }} key-suffix: ${{ inputs.cache-build-image-key-suffix }} - - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 id: appauth name: Appauth if: ${{ inputs.trusted }} @@ -221,7 +224,7 @@ jobs: # - the workaround is to allow the token to be passed through. token: ${{ github.token }} token-ok: true - - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.36 id: checkout name: Checkout Envoy repository with: @@ -238,7 +241,7 @@ jobs: token: ${{ inputs.trusted && steps.appauth.outputs.token || github.token }} # This is currently only use by mobile-docs and can be removed once they are updated to the newer website - - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.36 id: checkout-extra name: Checkout extra repository (for publishing) if: ${{ inputs.checkout-extra }} @@ -247,7 +250,7 @@ jobs: ssh-key: ${{ inputs.trusted && inputs.ssh-key-extra || '' }} - name: Import GPG key - uses: envoyproxy/toolshed/gh-actions/gpg/import@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/gpg/import@actions-v0.2.36 if: ${{ inputs.import-gpg }} with: key: ${{ secrets.gpg-key }} @@ -276,11 +279,14 @@ jobs: GCP_SERVICE_ACCOUNT_KEY_PATH=$(mktemp -p "${{ runner.temp }}" -t gcp_service_account.XXXXXX.json) echo "${{ secrets.gcp-key }}" | base64 --decode > "${GCP_SERVICE_ACCOUNT_KEY_PATH}" GCP_SERVICE_ACCOUNT_KEY_FILE="$(basename "${GCP_SERVICE_ACCOUNT_KEY_PATH}")" + echo "GCP_SERVICE_ACCOUNT_KEY_PATH=/build/${GCP_SERVICE_ACCOUNT_KEY_FILE}" >> "$GITHUB_ENV" + if [[ "${{ inputs.gcs-only }}" != "" ]]; then + exit 0 + fi BAZEL_BUILD_EXTRA_OPTIONS="--google_credentials=/build/${GCP_SERVICE_ACCOUNT_KEY_FILE} --config=remote-ci --config=rbe-google" echo "BAZEL_BUILD_EXTRA_OPTIONS=${BAZEL_BUILD_EXTRA_OPTIONS}" >> "$GITHUB_ENV" - echo "GCP_SERVICE_ACCOUNT_KEY_PATH=${GCP_SERVICE_ACCOUNT_KEY_PATH}" >> "$GITHUB_ENV" - - uses: envoyproxy/toolshed/gh-actions/github/run@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/run@actions-v0.2.36 name: Run CI ${{ inputs.command }} ${{ inputs.target }} with: args: ${{ inputs.args != '--' && inputs.args || inputs.target }} @@ -297,6 +303,11 @@ jobs: fail-match: ${{ inputs.fail-match }} notice-match: ${{ inputs.notice-match }} output-path: ${{ inputs.output-path }} + report-name: >- + ci-report-${{ + inputs.target-suffix + && format('{0}-', inputs.target-suffix) + || '' }}${{ inputs.target-name || inputs.target }}.json report-pre: ${{ inputs.report-pre }} report-post: ${{ inputs.report-post }} source: ${{ inputs.source }} diff --git a/.github/workflows/_start.yml b/.github/workflows/_start.yml index 9b475214da..a5c32521f9 100644 --- a/.github/workflows/_start.yml +++ b/.github/workflows/_start.yml @@ -54,7 +54,7 @@ jobs: start: runs-on: ubuntu-22.04 steps: - - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/jq@actions-v0.2.36 id: check-config name: Prepare check data with: @@ -77,13 +77,13 @@ jobs: | .skipped.output.summary = "${{ inputs.skipped-summary }}" | .skipped.output.text = "" - - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 name: Appauth id: appauth with: app_id: ${{ secrets.app-id }} key: ${{ secrets.app-key }} - - uses: envoyproxy/toolshed/gh-actions/github/checks@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/checks@actions-v0.2.36 name: Start checks id: checks with: @@ -94,7 +94,7 @@ jobs: ${{ fromJSON(inputs.env).summary.summary }} token: ${{ steps.appauth.outputs.token }} - - uses: envoyproxy/toolshed/gh-actions/json/table@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/json/table@actions-v0.2.36 name: Summary with: collapse-open: true @@ -118,7 +118,7 @@ jobs: output-path: GITHUB_STEP_SUMMARY title: Checks started/skipped - - uses: envoyproxy/toolshed/gh-actions/github/env/save@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/env/save@actions-v0.2.36 name: Save env id: data with: diff --git a/.github/workflows/codeql-daily.yml b/.github/workflows/codeql-daily.yml index 15562a0199..5101ab9d19 100644 --- a/.github/workflows/codeql-daily.yml +++ b/.github/workflows/codeql-daily.yml @@ -30,7 +30,7 @@ jobs: uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 - name: Free disk space - uses: envoyproxy/toolshed/gh-actions/diskspace@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/diskspace@actions-v0.2.36 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL diff --git a/.github/workflows/codeql-push.yml b/.github/workflows/codeql-push.yml index 10e4eb95e3..ff5dad1275 100644 --- a/.github/workflows/codeql-push.yml +++ b/.github/workflows/codeql-push.yml @@ -61,7 +61,7 @@ jobs: - name: Free disk space if: ${{ env.BUILD_TARGETS != '' }} - uses: envoyproxy/toolshed/gh-actions/diskspace@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/diskspace@actions-v0.2.36 - name: Initialize CodeQL if: ${{ env.BUILD_TARGETS != '' }} diff --git a/.github/workflows/command.yml b/.github/workflows/command.yml index 99f78cc63d..15109d4c29 100644 --- a/.github/workflows/command.yml +++ b/.github/workflows/command.yml @@ -28,7 +28,7 @@ jobs: && github.actor != 'dependabot[bot]' }} steps: - - uses: envoyproxy/toolshed/gh-actions/github/command@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/command@actions-v0.2.36 name: Parse command from comment id: command with: @@ -37,14 +37,14 @@ jobs: ^/(retest) # /retest - - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 if: ${{ steps.command.outputs.command == 'retest' }} id: appauth-retest name: Appauth (retest) with: key: ${{ secrets.ENVOY_CI_APP_KEY }} app_id: ${{ secrets.ENVOY_CI_APP_ID }} - - uses: envoyproxy/toolshed/gh-actions/retest@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/retest@actions-v0.2.36 if: ${{ steps.command.outputs.command == 'retest' }} name: Retest with: diff --git a/.github/workflows/envoy-checks.yml b/.github/workflows/envoy-checks.yml index ee6f9d94d5..08422f5ad5 100644 --- a/.github/workflows/envoy-checks.yml +++ b/.github/workflows/envoy-checks.yml @@ -59,7 +59,7 @@ jobs: coverage: secrets: - gcp-key: ${{ secrets.GCP_SERVICE_ACCOUNT_KEY }} + gcp-key: ${{ fromJSON(needs.load.outputs.trusted) && secrets.GCP_SERVICE_ACCOUNT_KEY_TRUSTED || secrets.GCP_SERVICE_ACCOUNT_KEY }} permissions: actions: read contents: read diff --git a/.github/workflows/envoy-dependency.yml b/.github/workflows/envoy-dependency.yml index 916894ac4c..933dad4218 100644 --- a/.github/workflows/envoy-dependency.yml +++ b/.github/workflows/envoy-dependency.yml @@ -53,16 +53,16 @@ jobs: steps: - id: appauth name: Appauth - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 with: app_id: ${{ secrets.ENVOY_CI_DEP_APP_ID }} key: ${{ secrets.ENVOY_CI_DEP_APP_KEY }} - id: checkout name: Checkout Envoy repository - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.36 with: token: ${{ steps.appauth.outputs.token }} - - uses: envoyproxy/toolshed/gh-actions/bson@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/bson@actions-v0.2.36 id: update name: Update dependency (${{ inputs.dependency }}) with: @@ -97,13 +97,13 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - uses: envoyproxy/toolshed/gh-actions/upload/diff@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/upload/diff@actions-v0.2.36 name: Upload diff with: name: ${{ inputs.dependency }}-${{ steps.update.outputs.output }} - name: Create a PR if: ${{ inputs.pr }} - uses: envoyproxy/toolshed/gh-actions/github/pr@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/github/pr@actions-v0.2.36 with: base: main body: | @@ -134,11 +134,11 @@ jobs: steps: - id: appauth name: Appauth - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 with: app_id: ${{ secrets.ENVOY_CI_DEP_APP_ID }} key: ${{ secrets.ENVOY_CI_DEP_APP_KEY }} - - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.36 id: checkout name: Checkout Envoy repository with: @@ -180,7 +180,7 @@ jobs: - name: Check Docker SHAs id: build-images - uses: envoyproxy/toolshed/gh-actions/docker/shas@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/docker/shas@actions-v0.2.36 with: images: | sha: envoyproxy/envoy-build-ubuntu:${{ steps.build-tools.outputs.tag }} @@ -209,7 +209,7 @@ jobs: name: Update SHAs working-directory: envoy - name: Create a PR - uses: envoyproxy/toolshed/gh-actions/github/pr@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/github/pr@actions-v0.2.36 with: base: main body: Created by Envoy dependency bot diff --git a/.github/workflows/envoy-macos.yml b/.github/workflows/envoy-macos.yml index 9182073581..9859cab5aa 100644 --- a/.github/workflows/envoy-macos.yml +++ b/.github/workflows/envoy-macos.yml @@ -52,6 +52,7 @@ jobs: steps-post: steps-pre: ${{ matrix.steps-pre }} target: ${{ matrix.target }} + target-name: ${{ matrix.target-name }} timeout-minutes: 90 trusted: ${{ fromJSON(needs.load.outputs.trusted) }} strategy: @@ -60,6 +61,7 @@ jobs: include: - target: ci/mac_ci_steps.sh name: macOS + target-name: mac_ci_steps source: | source ./ci/mac_ci_setup.sh _BAZEL_BUILD_EXTRA_OPTIONS=( diff --git a/.github/workflows/envoy-prechecks.yml b/.github/workflows/envoy-prechecks.yml index 8608560813..35e9eed6fe 100644 --- a/.github/workflows/envoy-prechecks.yml +++ b/.github/workflows/envoy-prechecks.yml @@ -67,6 +67,8 @@ jobs: trusted: ${{ fromJSON(needs.load.outputs.trusted) }} publish: + secrets: + gcp-key: ${{ fromJSON(needs.load.outputs.trusted) && secrets.GCP_SERVICE_ACCOUNT_KEY_TRUSTED || secrets.GCP_SERVICE_ACCOUNT_KEY }} permissions: actions: read contents: read diff --git a/.github/workflows/envoy-release.yml b/.github/workflows/envoy-release.yml index 104df712e7..865b7f42ff 100644 --- a/.github/workflows/envoy-release.yml +++ b/.github/workflows/envoy-release.yml @@ -55,14 +55,14 @@ jobs: steps: - id: appauth name: App auth - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 with: app_id: ${{ secrets.ENVOY_CI_PUBLISH_APP_ID }} key: ${{ secrets.ENVOY_CI_PUBLISH_APP_KEY }} - id: checkout name: Checkout Envoy repository - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.36 with: committer-name: ${{ env.COMMITTER_NAME }} committer-email: ${{ env.COMMITTER_EMAIL }} @@ -83,10 +83,10 @@ jobs: name: Check changelog summary - if: ${{ inputs.author }} name: Validate signoff email - uses: envoyproxy/toolshed/gh-actions/email/validate@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/email/validate@actions-v0.2.36 with: email: ${{ inputs.author }} - - uses: envoyproxy/toolshed/gh-actions/github/run@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/run@actions-v0.2.36 name: Create release with: source: | @@ -111,7 +111,7 @@ jobs: name: Release version id: release - name: Create a PR - uses: envoyproxy/toolshed/gh-actions/github/pr@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/github/pr@actions-v0.2.36 with: base: ${{ github.ref_name }} commit: false @@ -136,20 +136,20 @@ jobs: steps: - id: appauth name: App auth - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 with: app_id: ${{ secrets.ENVOY_CI_PUBLISH_APP_ID }} key: ${{ secrets.ENVOY_CI_PUBLISH_APP_KEY }} - id: checkout name: Checkout Envoy repository - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.36 with: committer-name: ${{ env.COMMITTER_NAME }} committer-email: ${{ env.COMMITTER_EMAIL }} strip-prefix: release/ token: ${{ steps.appauth.outputs.token }} - - uses: envoyproxy/toolshed/gh-actions/github/run@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/github/run@actions-v0.2.36 name: Sync version histories with: command: >- @@ -159,7 +159,7 @@ jobs: -- --signoff="${{ env.COMMITTER_NAME }} <${{ env.COMMITTER_EMAIL }}>" - name: Create a PR - uses: envoyproxy/toolshed/gh-actions/github/pr@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/github/pr@actions-v0.2.36 with: append-commit-message: true base: ${{ github.ref_name }} @@ -189,13 +189,13 @@ jobs: steps: - id: appauth name: App auth - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 with: app_id: ${{ secrets.ENVOY_CI_PUBLISH_APP_ID }} key: ${{ secrets.ENVOY_CI_PUBLISH_APP_KEY }} - name: Checkout repository - uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/github/checkout@actions-v0.2.36 with: committer-name: ${{ env.COMMITTER_NAME }} committer-email: ${{ env.COMMITTER_EMAIL }} diff --git a/.github/workflows/envoy-sync.yml b/.github/workflows/envoy-sync.yml index 197387b0ff..1858675831 100644 --- a/.github/workflows/envoy-sync.yml +++ b/.github/workflows/envoy-sync.yml @@ -34,12 +34,12 @@ jobs: - data-plane-api - mobile-website steps: - - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 id: appauth with: app_id: ${{ secrets.ENVOY_CI_SYNC_APP_ID }} key: ${{ secrets.ENVOY_CI_SYNC_APP_KEY }} - - uses: envoyproxy/toolshed/gh-actions/dispatch@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/dispatch@actions-v0.2.36 with: repository: "envoyproxy/${{ matrix.downstream }}" ref: main @@ -61,12 +61,12 @@ jobs: downstream: - envoy-openssl steps: - - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/appauth@actions-v0.2.36 id: appauth with: app_id: ${{ secrets.ENVOY_CI_SYNC_APP_ID }} key: ${{ secrets.ENVOY_CI_SYNC_APP_KEY }} - - uses: envoyproxy/toolshed/gh-actions/dispatch@actions-v0.2.35 + - uses: envoyproxy/toolshed/gh-actions/dispatch@actions-v0.2.36 with: repository: "envoyproxy/${{ matrix.downstream }}" ref: release/v1.28 diff --git a/.github/workflows/garbage.yml b/.github/workflows/garbage.yml index 01783dfd5a..d6e0af919e 100644 --- a/.github/workflows/garbage.yml +++ b/.github/workflows/garbage.yml @@ -33,7 +33,7 @@ jobs: pool-id: 17 steps: - name: Remove dead AZP agents (${{ matrix.target }}) - uses: envoyproxy/toolshed/gh-actions/azp/agent-cleanup@actions-v0.2.35 + uses: envoyproxy/toolshed/gh-actions/azp/agent-cleanup@actions-v0.2.36 with: azp-org: cncf azp-token: ${{ secrets.AZP_TOKEN }} diff --git a/api/envoy/extensions/filters/http/ext_proc/v3/BUILD b/api/envoy/extensions/filters/http/ext_proc/v3/BUILD index 8322f99fa7..5bfeeda1b7 100644 --- a/api/envoy/extensions/filters/http/ext_proc/v3/BUILD +++ b/api/envoy/extensions/filters/http/ext_proc/v3/BUILD @@ -10,5 +10,6 @@ api_proto_package( "//envoy/config/core/v3:pkg", "//envoy/type/matcher/v3:pkg", "@com_github_cncf_xds//udpa/annotations:pkg", + "@com_github_cncf_xds//xds/annotations/v3:pkg", ], ) diff --git a/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto b/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto index 13a24ad9fc..83b15731b9 100644 --- a/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto +++ b/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto @@ -12,6 +12,8 @@ import "envoy/type/matcher/v3/string.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/struct.proto"; +import "xds/annotations/v3/status.proto"; + import "udpa/annotations/migrate.proto"; import "udpa/annotations/status.proto"; import "validate/validate.proto"; @@ -131,8 +133,39 @@ message ExternalProcessor { // Only one of ``http_service`` or // :ref:`grpc_service `. // can be set. It is required that one of them must be set. - ExtProcHttpService http_service = 20 - [(udpa.annotations.field_migrate).oneof_promotion = "ext_proc_service_type"]; + // + // If ``http_service`` is set, the + // :ref:`processing_mode ` + // can not be configured to send any body or trailers. i.e, http_service only supports + // sending request or response headers to the side stream server. + // + // With this configuration, Envoy behavior: + // + // 1. The headers are first put in a proto message + // :ref:`ProcessingRequest `. + // + // 2. This proto message is then transcoded into a JSON text. + // + // 3. Envoy then sends a HTTP POST message with content-type as "application/json", + // and this JSON text as body to the side stream server. + // + // After the side-stream receives this HTTP request message, it is expected to do as follows: + // + // 1. It converts the body, which is a JSON string, into a ``ProcessingRequest`` + // proto message to examine and mutate the headers. + // + // 2. It then sets the mutated headers into a new proto message + // :ref:`ProcessingResponse `. + // + // 3. It converts ``ProcessingResponse`` proto message into a JSON text. + // + // 4. It then sends a HTTP response back to Envoy with status code as "200", + // content-type as "application/json" and sets the JSON text as the body. + // + ExtProcHttpService http_service = 20 [ + (udpa.annotations.field_migrate).oneof_promotion = "ext_proc_service_type", + (xds.annotations.v3.field_status).work_in_progress = true + ]; // By default, if the gRPC stream cannot be established, or if it is closed // prematurely with an error, the filter will fail. Specifically, if the diff --git a/bazel/external/quiche.BUILD b/bazel/external/quiche.BUILD index ccd6261ad6..3a334c4d4e 100644 --- a/bazel/external/quiche.BUILD +++ b/bazel/external/quiche.BUILD @@ -400,12 +400,12 @@ envoy_cc_library( copts = quiche_copts, repository = "@envoy", deps = [ + ":common_http_http_header_block_lib", ":http2_adapter_data_source", ":http2_adapter_http2_protocol", ":http2_adapter_http2_visitor_interface", ":http2_adapter_nghttp2_include", ":quiche_common_platform_export", - ":spdy_core_http2_header_block_lib", ], ) @@ -435,6 +435,7 @@ envoy_cc_library( copts = quiche_copts, repository = "@envoy", deps = [ + ":common_http_http_header_block_lib", ":http2_adapter_chunked_buffer", ":http2_adapter_data_source", ":http2_adapter_event_forwarder", @@ -453,7 +454,6 @@ envoy_cc_library( ":http2_header_byte_listener_interface_lib", ":http2_no_op_headers_handler_lib", ":quiche_common_callbacks", - ":spdy_core_http2_header_block_lib", "@com_google_absl//absl/algorithm", "@com_google_absl//absl/cleanup", ], @@ -498,9 +498,9 @@ envoy_cc_library( copts = quiche_copts, repository = "@envoy", deps = [ + ":common_http_http_header_block_lib", ":http2_adapter_http2_protocol", ":quiche_common_platform_export", - ":spdy_core_http2_header_block_lib", ], ) @@ -570,6 +570,7 @@ envoy_cc_test_library( copts = quiche_copts, repository = "@envoy", deps = [ + ":common_http_http_header_block_lib", ":http2_adapter_chunked_buffer", ":http2_adapter_data_source", ":http2_adapter_http2_protocol", @@ -578,7 +579,6 @@ envoy_cc_test_library( ":http2_core_protocol_lib", ":http2_hpack_hpack_lib", ":quiche_common_platform_test", - ":spdy_core_http2_header_block_lib", ], ) @@ -1320,7 +1320,6 @@ envoy_cc_library( srcs = ["quiche/http2/core/spdy_alt_svc_wire_format.cc"], hdrs = [ "quiche/http2/core/spdy_alt_svc_wire_format.h", - "quiche/spdy/core/spdy_alt_svc_wire_format.h", ], copts = quiche_copts, repository = "@envoy", @@ -1337,25 +1336,23 @@ envoy_cc_library( hdrs = [ "quiche/http2/core/spdy_frame_builder.h", "quiche/http2/core/spdy_framer.h", - "quiche/spdy/core/spdy_frame_builder.h", - "quiche/spdy/core/spdy_framer.h", ], copts = quiche_copts, repository = "@envoy", deps = [ + ":common_http_http_header_block_lib", ":http2_core_alt_svc_wire_format_lib", ":http2_core_headers_handler_interface_lib", ":http2_core_protocol_lib", ":http2_core_zero_copy_output_buffer_lib", ":http2_hpack_hpack_lib", ":quiche_common_platform", - ":spdy_core_http2_header_block_lib", ], ) envoy_cc_library( - name = "spdy_core_http2_header_block_lib", - hdrs = ["quiche/spdy/core/http2_header_block.h"], + name = "common_http_http_header_block_lib", + hdrs = ["quiche/common/http/http_header_block.h"], copts = quiche_copts, repository = "@envoy", visibility = ["//visibility:public"], @@ -1381,13 +1378,11 @@ envoy_cc_library( envoy_cc_library( name = "http2_core_http2_deframer_lib", srcs = ["quiche/http2/core/http2_frame_decoder_adapter.cc"], - hdrs = [ - "quiche/http2/core/http2_frame_decoder_adapter.h", - "quiche/spdy/core/http2_frame_decoder_adapter.h", - ], + hdrs = ["quiche/http2/core/http2_frame_decoder_adapter.h"], copts = quiche_copts, repository = "@envoy", deps = [ + ":common_http_http_header_block_lib", ":http2_constants_lib", ":http2_core_alt_svc_wire_format_lib", ":http2_core_headers_handler_interface_lib", @@ -1396,11 +1391,10 @@ envoy_cc_library( ":http2_decoder_decode_status_lib", ":http2_decoder_frame_decoder_lib", ":http2_decoder_frame_decoder_listener_lib", + ":http2_hpack_hpack_decoder_adapter_lib", ":http2_hpack_hpack_lib", ":http2_structures_lib", ":quiche_common_platform", - ":spdy_core_hpack_hpack_decoder_adapter_lib", - ":spdy_core_http2_header_block_lib", ], ) @@ -1427,7 +1421,6 @@ envoy_cc_library( "quiche/http2/hpack/hpack_header_table.h", "quiche/http2/hpack/hpack_output_stream.h", "quiche/http2/hpack/hpack_static_table.h", - "quiche/spdy/core/hpack/hpack_encoder.h", ], copts = quiche_copts, repository = "@envoy", @@ -1442,12 +1435,13 @@ envoy_cc_library( ) envoy_cc_library( - name = "spdy_core_hpack_hpack_decoder_adapter_lib", - srcs = ["quiche/spdy/core/hpack/hpack_decoder_adapter.cc"], - hdrs = ["quiche/spdy/core/hpack/hpack_decoder_adapter.h"], + name = "http2_hpack_hpack_decoder_adapter_lib", + srcs = ["quiche/http2/hpack/hpack_decoder_adapter.cc"], + hdrs = ["quiche/http2/hpack/hpack_decoder_adapter.h"], copts = quiche_copts, repository = "@envoy", deps = [ + ":common_http_http_header_block_lib", ":http2_core_headers_handler_interface_lib", ":http2_decoder_decode_buffer_lib", ":http2_decoder_decode_status_lib", @@ -1458,7 +1452,6 @@ envoy_cc_library( ":http2_hpack_hpack_lib", ":http2_no_op_headers_handler_lib", ":quiche_common_platform", - ":spdy_core_http2_header_block_lib", ], ) @@ -1482,16 +1475,14 @@ envoy_cc_library( hdrs = [ "quiche/http2/core/spdy_bitmasks.h", "quiche/http2/core/spdy_protocol.h", - "quiche/spdy/core/spdy_bitmasks.h", - "quiche/spdy/core/spdy_protocol.h", ], copts = quiche_copts, repository = "@envoy", visibility = ["//visibility:public"], deps = [ + ":common_http_http_header_block_lib", ":http2_core_alt_svc_wire_format_lib", ":quiche_common_platform", - ":spdy_core_http2_header_block_lib", ], ) @@ -1501,8 +1492,8 @@ envoy_cc_library( hdrs = ["quiche/http2/core/recording_headers_handler.h"], repository = "@envoy", deps = [ + ":common_http_http_header_block_lib", ":http2_core_headers_handler_interface_lib", - ":spdy_core_http2_header_block_lib", ], ) @@ -1513,11 +1504,11 @@ envoy_cc_test_library( copts = quiche_copts, repository = "@envoy", deps = [ + ":common_http_http_header_block_lib", ":http2_core_headers_handler_interface_lib", ":http2_core_protocol_lib", ":quiche_common_platform", ":quiche_common_test_tools_test_utils_lib", - ":spdy_core_http2_header_block_lib", ], ) @@ -3088,13 +3079,13 @@ envoy_quic_cc_library( srcs = ["quiche/quic/core/http/quic_header_list.cc"], hdrs = ["quiche/quic/core/http/quic_header_list.h"], deps = [ + ":common_http_http_header_block_lib", ":http2_core_headers_handler_interface_lib", ":http2_core_protocol_lib", ":quic_core_packets_lib", ":quic_core_qpack_qpack_header_table_lib", ":quic_platform_base", ":quiche_common_circular_deque_lib", - ":spdy_core_http2_header_block_lib", ], ) @@ -3830,8 +3821,8 @@ envoy_quic_cc_library( srcs = ["quiche/quic/core/qpack/value_splitting_header_list.cc"], hdrs = ["quiche/quic/core/qpack/value_splitting_header_list.h"], deps = [ + ":common_http_http_header_block_lib", ":quic_platform_base", - ":spdy_core_http2_header_block_lib", ], ) @@ -5600,10 +5591,10 @@ envoy_cc_library( repository = "@envoy", tags = ["nofips"], deps = [ + ":common_http_http_header_block_lib", ":quiche_common_callbacks", ":quiche_common_platform_export", ":quiche_common_quiche_stream_lib", - ":spdy_core_http2_header_block_lib", "@com_google_absl//absl/strings", "@com_google_absl//absl/time", "@com_google_absl//absl/types:span", diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 5eada04195..b5233988c6 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -1208,12 +1208,12 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "QUICHE", project_desc = "QUICHE (QUIC, HTTP/2, Etc) is Google‘s implementation of QUIC and related protocols", project_url = "https://github.com/google/quiche", - version = "42b2e66c721f442bb439b40a1e037897360cf1b2", - sha256 = "f72f78d7fa57154ad302d559fee6b72e0695d51391684891ec991b2b5d90491f", + version = "171f6f89a6a119e8763f1216f8d85347f997cd3b", + sha256 = "3e0fec32dfa9c7568d4703516ee14c9e2316379e0a35f723d17a988be178e532", urls = ["https://github.com/google/quiche/archive/{version}.tar.gz"], strip_prefix = "quiche-{version}", use_category = ["controlplane", "dataplane_core"], - release_date = "2024-09-17", + release_date = "2024-09-26", cpe = "N/A", license = "BSD-3-Clause", license_url = "https://github.com/google/quiche/blob/{version}/LICENSE", diff --git a/ci/do_ci.sh b/ci/do_ci.sh index f67bbca4fd..99f29f661e 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -57,7 +57,6 @@ FETCH_PROTO_TARGETS=( @com_github_bufbuild_buf//:bin/buf //tools/proto_format/...) -GCS_REDIRECT_PATH="${SYSTEM_PULLREQUEST_PULLREQUESTNUMBER:-${BUILD_SOURCEBRANCHNAME}}" retry () { local n wait iterations diff --git a/ci/run_envoy_docker.sh b/ci/run_envoy_docker.sh index dcb86af9db..2d60de2f08 100755 --- a/ci/run_envoy_docker.sh +++ b/ci/run_envoy_docker.sh @@ -158,15 +158,14 @@ docker run --rm \ -e ENVOY_REPO \ -e ENVOY_TARBALL_DIR \ -e ENVOY_GEN_COMPDB_OPTIONS \ - -e SYSTEM_PULLREQUEST_PULLREQUESTNUMBER \ -e GCS_ARTIFACT_BUCKET \ + -e GCS_REDIRECT_PATH \ -e GITHUB_REF_NAME \ -e GITHUB_REF_TYPE \ -e GITHUB_TOKEN \ -e GITHUB_APP_ID \ -e GITHUB_INSTALL_ID \ -e MOBILE_DOCS_CHECKOUT_DIR \ - -e BUILD_SOURCEBRANCHNAME \ -e BAZELISK_BASE_URL \ -e ENVOY_BUILD_ARCH \ -e SYSTEM_STAGEDISPLAYNAME \ diff --git a/envoy/config/grpc_mux.h b/envoy/config/grpc_mux.h index a18c87bc19..096c439b0b 100644 --- a/envoy/config/grpc_mux.h +++ b/envoy/config/grpc_mux.h @@ -2,10 +2,13 @@ #include +#include "envoy/common/backoff_strategy.h" #include "envoy/common/exception.h" #include "envoy/common/pure.h" +#include "envoy/config/custom_config_validators.h" #include "envoy/config/eds_resources_cache.h" #include "envoy/config/subscription.h" +#include "envoy/grpc/async_client.h" #include "envoy/stats/stats_macros.h" #include "source/common/common/cleanup.h" @@ -112,6 +115,16 @@ class GrpcMux { * @return EdsResourcesCacheOptRef optional eds resources cache for the gRPC-mux. */ virtual EdsResourcesCacheOptRef edsResourcesCache() PURE; + + /** + * Updates the current gRPC-Mux object to use a new gRPC client, and config. + */ + virtual absl::Status + updateMuxSource(Grpc::RawAsyncClientPtr&& primary_async_client, + Grpc::RawAsyncClientPtr&& failover_async_client, + CustomConfigValidatorsPtr&& custom_config_validators, Stats::Scope& scope, + BackOffStrategyPtr&& backoff_strategy, + const envoy::config::core::v3::ApiConfigSource& ads_config_source) PURE; }; using GrpcMuxPtr = std::unique_ptr; diff --git a/mobile/library/cc/engine_builder.cc b/mobile/library/cc/engine_builder.cc index 53518b8cd9..8815ab4315 100644 --- a/mobile/library/cc/engine_builder.cc +++ b/mobile/library/cc/engine_builder.cc @@ -222,8 +222,8 @@ EngineBuilder& EngineBuilder::addQuicCanonicalSuffix(std::string suffix) { return *this; } -EngineBuilder& EngineBuilder::enablePortMigration(bool enable_port_migration) { - enable_port_migration_ = enable_port_migration; +EngineBuilder& EngineBuilder::setNumTimeoutsToTriggerPortMigration(int num_timeouts) { + num_timeouts_to_trigger_port_migration_ = num_timeouts; return *this; } @@ -267,6 +267,12 @@ EngineBuilder& EngineBuilder::setUpstreamTlsSni(std::string sni) { return *this; } +EngineBuilder& +EngineBuilder::setQuicConnectionIdleTimeoutSeconds(int quic_connection_idle_timeout_seconds) { + quic_connection_idle_timeout_seconds_ = quic_connection_idle_timeout_seconds; + return *this; +} + EngineBuilder& EngineBuilder::enablePlatformCertificatesValidation(bool platform_certificates_validation_on) { platform_certificates_validation_on_ = platform_certificates_validation_on; @@ -727,19 +733,19 @@ std::unique_ptr EngineBuilder::generate ->add_canonical_suffixes(suffix); } - if (enable_port_migration_) { + if (num_timeouts_to_trigger_port_migration_ > 0) { alpn_options.mutable_auto_config() ->mutable_http3_protocol_options() ->mutable_quic_protocol_options() ->mutable_num_timeouts_to_trigger_port_migration() - ->set_value(4); + ->set_value(num_timeouts_to_trigger_port_migration_); } alpn_options.mutable_auto_config() ->mutable_http3_protocol_options() ->mutable_quic_protocol_options() ->mutable_idle_network_timeout() - ->set_seconds(30); + ->set_seconds(quic_connection_idle_timeout_seconds_); base_cluster->mutable_transport_socket()->mutable_typed_config()->PackFrom(h3_proxy_socket); (*base_cluster->mutable_typed_extension_protocol_options()) diff --git a/mobile/library/cc/engine_builder.h b/mobile/library/cc/engine_builder.h index 7636eab146..0decc13dd7 100644 --- a/mobile/library/cc/engine_builder.h +++ b/mobile/library/cc/engine_builder.h @@ -62,7 +62,8 @@ class EngineBuilder { EngineBuilder& setHttp3ClientConnectionOptions(std::string options); EngineBuilder& addQuicHint(std::string host, int port); EngineBuilder& addQuicCanonicalSuffix(std::string suffix); - EngineBuilder& enablePortMigration(bool enable_port_migration); + // 0 means port migration is disabled. + EngineBuilder& setNumTimeoutsToTriggerPortMigration(int num_timeouts); EngineBuilder& enableInterfaceBinding(bool interface_binding_on); EngineBuilder& enableDrainPostDnsRefresh(bool drain_post_dns_refresh_on); // Sets whether to use GRO for upstream UDP sockets (QUIC/HTTP3). @@ -104,6 +105,9 @@ class EngineBuilder { // outside of this range will be ignored. EngineBuilder& setNetworkThreadPriority(int thread_priority); + // Sets the QUIC connection idle timeout in seconds. + EngineBuilder& setQuicConnectionIdleTimeoutSeconds(int quic_connection_idle_timeout_seconds); + #if defined(__APPLE__) // Right now, this API is only used by Apple (iOS) to register the Apple proxy resolver API for // use in reading and using the system proxy settings. @@ -176,7 +180,7 @@ class EngineBuilder { std::string http3_client_connection_options_ = ""; std::vector> quic_hints_; std::vector quic_suffixes_; - bool enable_port_migration_ = false; + int num_timeouts_to_trigger_port_migration_ = 0; bool always_use_v6_ = false; #if defined(__APPLE__) // TODO(abeyad): once stable, consider setting the default to true. @@ -201,6 +205,8 @@ class EngineBuilder { // https://source.chromium.org/chromium/chromium/src/+/main:net/quic/quic_session_pool.cc;l=790-793;drc=7f04a8e033c23dede6beae129cd212e6d4473d72 // https://source.chromium.org/chromium/chromium/src/+/main:net/third_party/quiche/src/quiche/quic/core/quic_constants.h;l=43-47;drc=34ad7f3844f882baf3d31a6bc6e300acaa0e3fc8 int32_t udp_socket_send_buffer_size_ = 1452 * 20; + + int quic_connection_idle_timeout_seconds_ = 30; }; using EngineBuilderSharedPtr = std::shared_ptr; diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java index 40f6f4dfc0..0cee67c995 100644 --- a/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java +++ b/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java @@ -46,7 +46,7 @@ public enum TrustChainVerification { public final List quicCanonicalSuffixes; public final boolean enableGzipDecompression; public final boolean enableBrotliDecompression; - public final boolean enablePortMigration; + public final int numTimeoutsToTriggerPortMigration; public final boolean enableSocketTagging; public final boolean enableInterfaceBinding; public final int h2ConnectionKeepaliveIdleIntervalMilliseconds; @@ -106,7 +106,8 @@ public enum TrustChainVerification { * decompression. * @param enableBrotliDecompression whether to enable response brotli * decompression. - * @param enablePortMigration whether to enable quic port migration. + * @param numTimeoutsToTriggerPortMigration number of timeouts to trigger port + * migration. * @param enableSocketTagging whether to enable socket tagging. * @param enableInterfaceBinding whether to allow interface binding. * @param h2ConnectionKeepaliveIdleIntervalMilliseconds rate in milliseconds seconds to send h2 @@ -139,11 +140,11 @@ public EnvoyConfiguration( boolean forceV6, boolean useGro, String http3ConnectionOptions, String http3ClientConnectionOptions, Map quicHints, List quicCanonicalSuffixes, boolean enableGzipDecompression, - boolean enableBrotliDecompression, boolean enablePortMigration, boolean enableSocketTagging, - boolean enableInterfaceBinding, int h2ConnectionKeepaliveIdleIntervalMilliseconds, - int h2ConnectionKeepaliveTimeoutSeconds, int maxConnectionsPerHost, - int streamIdleTimeoutSeconds, int perTryIdleTimeoutSeconds, String appVersion, String appId, - TrustChainVerification trustChainVerification, + boolean enableBrotliDecompression, int numTimeoutsToTriggerPortMigration, + boolean enableSocketTagging, boolean enableInterfaceBinding, + int h2ConnectionKeepaliveIdleIntervalMilliseconds, int h2ConnectionKeepaliveTimeoutSeconds, + int maxConnectionsPerHost, int streamIdleTimeoutSeconds, int perTryIdleTimeoutSeconds, + String appVersion, String appId, TrustChainVerification trustChainVerification, List nativeFilterChain, List httpPlatformFilterFactories, Map stringAccessors, @@ -180,7 +181,7 @@ public EnvoyConfiguration( this.quicCanonicalSuffixes = quicCanonicalSuffixes; this.enableGzipDecompression = enableGzipDecompression; this.enableBrotliDecompression = enableBrotliDecompression; - this.enablePortMigration = enablePortMigration; + this.numTimeoutsToTriggerPortMigration = numTimeoutsToTriggerPortMigration; this.enableSocketTagging = enableSocketTagging; this.enableInterfaceBinding = enableInterfaceBinding; this.h2ConnectionKeepaliveIdleIntervalMilliseconds = @@ -234,10 +235,11 @@ public long createBootstrap() { enableDNSCache, dnsCacheSaveIntervalSeconds, dnsNumRetries, enableDrainPostDnsRefresh, enableHttp3, useCares, forceV6, useGro, http3ConnectionOptions, http3ClientConnectionOptions, quicHints, quicSuffixes, enableGzipDecompression, - enableBrotliDecompression, enablePortMigration, enableSocketTagging, enableInterfaceBinding, - h2ConnectionKeepaliveIdleIntervalMilliseconds, h2ConnectionKeepaliveTimeoutSeconds, - maxConnectionsPerHost, streamIdleTimeoutSeconds, perTryIdleTimeoutSeconds, appVersion, - appId, enforceTrustChainVerification, filterChain, enablePlatformCertificatesValidation, - upstreamTlsSni, runtimeGuards, caresFallbackResolvers); + enableBrotliDecompression, numTimeoutsToTriggerPortMigration, enableSocketTagging, + enableInterfaceBinding, h2ConnectionKeepaliveIdleIntervalMilliseconds, + h2ConnectionKeepaliveTimeoutSeconds, maxConnectionsPerHost, streamIdleTimeoutSeconds, + perTryIdleTimeoutSeconds, appVersion, appId, enforceTrustChainVerification, filterChain, + enablePlatformCertificatesValidation, upstreamTlsSni, runtimeGuards, + caresFallbackResolvers); } } diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java index fc728abdba..2791ae0cd9 100644 --- a/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java +++ b/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java @@ -307,10 +307,11 @@ public static native long createBootstrap( boolean forceV6, boolean useGro, String http3ConnectionOptions, String http3ClientConnectionOptions, byte[][] quicHints, byte[][] quicCanonicalSuffixes, boolean enableGzipDecompression, boolean enableBrotliDecompression, - boolean enablePortMigration, boolean enableSocketTagging, boolean enableInterfaceBinding, - long h2ConnectionKeepaliveIdleIntervalMilliseconds, long h2ConnectionKeepaliveTimeoutSeconds, - long maxConnectionsPerHost, long streamIdleTimeoutSeconds, long perTryIdleTimeoutSeconds, - String appVersion, String appId, boolean trustChainVerification, byte[][] filterChain, + int numTimeoutsToTriggerPortMigration, boolean enableSocketTagging, + boolean enableInterfaceBinding, long h2ConnectionKeepaliveIdleIntervalMilliseconds, + long h2ConnectionKeepaliveTimeoutSeconds, long maxConnectionsPerHost, + long streamIdleTimeoutSeconds, long perTryIdleTimeoutSeconds, String appVersion, String appId, + boolean trustChainVerification, byte[][] filterChain, boolean enablePlatformCertificatesValidation, String upstreamTlsSni, byte[][] runtimeGuards, byte[][] cares_fallback_resolvers); diff --git a/mobile/library/java/org/chromium/net/impl/CronvoyEngineBuilderImpl.java b/mobile/library/java/org/chromium/net/impl/CronvoyEngineBuilderImpl.java index bbd867c55b..940eb12854 100644 --- a/mobile/library/java/org/chromium/net/impl/CronvoyEngineBuilderImpl.java +++ b/mobile/library/java/org/chromium/net/impl/CronvoyEngineBuilderImpl.java @@ -62,7 +62,7 @@ final static class Pkp { private String mQuicClientConnectionOptions = ""; private boolean mHttp2Enabled; private boolean mBrotiEnabled; - private boolean mPortMigrationEnabled; + private int mNumTimeoutsToTriggerPortMigration; private boolean mDisableCache; private int mHttpCacheMode; private long mHttpCacheMaxSize; @@ -240,12 +240,13 @@ public CronvoyEngineBuilderImpl addQuicCanonicalSuffix(String suffix) { List quicCanonicalSuffixes() { return mQuicCanonicalSuffixes; } - public CronvoyEngineBuilderImpl enablePortMigration(boolean enablePortMigration) { - mPortMigrationEnabled = enablePortMigration; + public CronvoyEngineBuilderImpl + setNumTimeoutsToTriggerPortMigration(int numTimeoutsToTriggerPortMigration) { + mNumTimeoutsToTriggerPortMigration = numTimeoutsToTriggerPortMigration; return this; } - boolean portMigrationEnabled() { return mPortMigrationEnabled; } + int numTimeoutsToTriggerPortMigration() { return mNumTimeoutsToTriggerPortMigration; } @Override public CronvoyEngineBuilderImpl addPublicKeyPins(String hostName, Set pinsSha256, diff --git a/mobile/library/java/org/chromium/net/impl/NativeCronvoyEngineBuilderImpl.java b/mobile/library/java/org/chromium/net/impl/NativeCronvoyEngineBuilderImpl.java index 8cdaed009a..4ff3415d67 100644 --- a/mobile/library/java/org/chromium/net/impl/NativeCronvoyEngineBuilderImpl.java +++ b/mobile/library/java/org/chromium/net/impl/NativeCronvoyEngineBuilderImpl.java @@ -291,11 +291,12 @@ private EnvoyConfiguration createEnvoyConfiguration() { mDnsPreresolveHostnames, mEnableDNSCache, mDnsCacheSaveIntervalSeconds, mDnsNumRetries.orElse(-1), mEnableDrainPostDnsRefresh, quicEnabled(), mUseCares, mForceV6, mUseGro, quicConnectionOptions(), quicClientConnectionOptions(), quicHints(), - quicCanonicalSuffixes(), mEnableGzipDecompression, brotliEnabled(), portMigrationEnabled(), - mEnableSocketTag, mEnableInterfaceBinding, mH2ConnectionKeepaliveIdleIntervalMilliseconds, - mH2ConnectionKeepaliveTimeoutSeconds, mMaxConnectionsPerHost, mStreamIdleTimeoutSeconds, - mPerTryIdleTimeoutSeconds, mAppVersion, mAppId, mTrustChainVerification, nativeFilterChain, - platformFilterChain, stringAccessors, keyValueStores, mRuntimeGuards, - mEnablePlatformCertificatesValidation, mUpstreamTlsSni, mCaresFallbackResolvers); + quicCanonicalSuffixes(), mEnableGzipDecompression, brotliEnabled(), + numTimeoutsToTriggerPortMigration(), mEnableSocketTag, mEnableInterfaceBinding, + mH2ConnectionKeepaliveIdleIntervalMilliseconds, mH2ConnectionKeepaliveTimeoutSeconds, + mMaxConnectionsPerHost, mStreamIdleTimeoutSeconds, mPerTryIdleTimeoutSeconds, mAppVersion, + mAppId, mTrustChainVerification, nativeFilterChain, platformFilterChain, stringAccessors, + keyValueStores, mRuntimeGuards, mEnablePlatformCertificatesValidation, mUpstreamTlsSni, + mCaresFallbackResolvers); } } diff --git a/mobile/library/jni/jni_impl.cc b/mobile/library/jni/jni_impl.cc index fd8d0de69c..ec987e863e 100644 --- a/mobile/library/jni/jni_impl.cc +++ b/mobile/library/jni/jni_impl.cc @@ -1222,8 +1222,9 @@ void configureBuilder(Envoy::JNI::JniHelper& jni_helper, jlong connect_timeout_s jboolean use_gro, jstring http3_connection_options, jstring http3_client_connection_options, jobjectArray quic_hints, jobjectArray quic_canonical_suffixes, jboolean enable_gzip_decompression, - jboolean enable_brotli_decompression, jboolean enable_port_migration, - jboolean enable_socket_tagging, jboolean enable_interface_binding, + jboolean enable_brotli_decompression, + jlong num_timeouts_to_trigger_port_migration, jboolean enable_socket_tagging, + jboolean enable_interface_binding, jlong h2_connection_keepalive_idle_interval_milliseconds, jlong h2_connection_keepalive_timeout_seconds, jlong max_connections_per_host, jlong stream_idle_timeout_seconds, jlong per_try_idle_timeout_seconds, @@ -1270,7 +1271,7 @@ void configureBuilder(Envoy::JNI::JniHelper& jni_helper, jlong connect_timeout_s for (const std::string& suffix : suffixes) { builder.addQuicCanonicalSuffix(suffix); } - builder.enablePortMigration(enable_port_migration); + builder.setNumTimeoutsToTriggerPortMigration(num_timeouts_to_trigger_port_migration); builder.setUseCares(use_cares == JNI_TRUE); if (use_cares == JNI_TRUE) { auto resolvers = javaObjectArrayToStringPairVector(jni_helper, cares_fallback_resolvers); @@ -1326,7 +1327,7 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr jboolean force_v6, jboolean use_gro, jstring http3_connection_options, jstring http3_client_connection_options, jobjectArray quic_hints, jobjectArray quic_canonical_suffixes, jboolean enable_gzip_decompression, - jboolean enable_brotli_decompression, jboolean enable_port_migration, + jboolean enable_brotli_decompression, jlong num_timeouts_to_trigger_port_migration, jboolean enable_socket_tagging, jboolean enable_interface_binding, jlong h2_connection_keepalive_idle_interval_milliseconds, jlong h2_connection_keepalive_timeout_seconds, jlong max_connections_per_host, @@ -1344,8 +1345,8 @@ extern "C" JNIEXPORT jlong JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibr enable_drain_post_dns_refresh, enable_http3, use_cares, force_v6, use_gro, http3_connection_options, http3_client_connection_options, quic_hints, quic_canonical_suffixes, enable_gzip_decompression, enable_brotli_decompression, - enable_port_migration, enable_socket_tagging, enable_interface_binding, - h2_connection_keepalive_idle_interval_milliseconds, + num_timeouts_to_trigger_port_migration, enable_socket_tagging, + enable_interface_binding, h2_connection_keepalive_idle_interval_milliseconds, h2_connection_keepalive_timeout_seconds, max_connections_per_host, stream_idle_timeout_seconds, per_try_idle_timeout_seconds, app_version, app_id, trust_chain_verification, filter_chain, enable_platform_certificates_validation, diff --git a/mobile/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt b/mobile/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt index bbc6ff7a51..ebf81c8711 100644 --- a/mobile/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt +++ b/mobile/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt @@ -49,7 +49,7 @@ open class EngineBuilder() { private var quicCanonicalSuffixes = mutableListOf() private var enableGzipDecompression = true private var enableBrotliDecompression = false - private var enablePortMigration = false + private var numTimeoutsToTriggerPortMigration = 0 private var enableSocketTagging = false private var enableInterfaceBinding = false private var h2ConnectionKeepaliveIdleIntervalMilliseconds = 1 @@ -268,13 +268,14 @@ open class EngineBuilder() { } /** - * Specify whether to do quic port migration or not. Defaults to false. + * Configure QUIC port migration. Defaults to disabled. * - * @param enablePortMigration whether or not to allow quic port migration. + * @param numTimeoutsToTriggerPortMigration number of timeouts to trigger port migration. If 0, + * port migration is disabled. * @return This builder. */ - fun enablePortMigration(enablePortMigration: Boolean): EngineBuilder { - this.enablePortMigration = enablePortMigration + fun setNumTimeoutsToTriggerPortMigration(numTimeoutsToTriggerPortMigration: Int): EngineBuilder { + this.numTimeoutsToTriggerPortMigration = numTimeoutsToTriggerPortMigration return this } @@ -578,7 +579,7 @@ open class EngineBuilder() { quicCanonicalSuffixes, enableGzipDecompression, enableBrotliDecompression, - enablePortMigration, + numTimeoutsToTriggerPortMigration, enableSocketTagging, enableInterfaceBinding, h2ConnectionKeepaliveIdleIntervalMilliseconds, diff --git a/mobile/test/cc/unit/envoy_config_test.cc b/mobile/test/cc/unit/envoy_config_test.cc index df94f22ffd..f0302a298d 100644 --- a/mobile/test/cc/unit/envoy_config_test.cc +++ b/mobile/test/cc/unit/envoy_config_test.cc @@ -71,7 +71,7 @@ TEST(TestConfig, ConfigIsApplied) { .addQuicHint("www.def.com", 443) .addQuicCanonicalSuffix(".opq.com") .addQuicCanonicalSuffix(".xyz.com") - .enablePortMigration(true) + .setNumTimeoutsToTriggerPortMigration(4) .addConnectTimeoutSeconds(123) .addDnsRefreshSeconds(456) .addDnsMinRefreshSeconds(567) diff --git a/mobile/test/common/internal_engine_test.cc b/mobile/test/common/internal_engine_test.cc index c15cf0c434..be479a821b 100644 --- a/mobile/test/common/internal_engine_test.cc +++ b/mobile/test/common/internal_engine_test.cc @@ -503,8 +503,8 @@ TEST_F(ThreadPriorityInternalEngineTest, SetThreadPriority) { } TEST_F(ThreadPriorityInternalEngineTest, SetOutOfRangeThreadPriority) { - // 42 is outside the range of acceptable thread priorities. - const int expected_thread_priority = 42; + // 102 is outside the range of acceptable thread priorities on all platforms. + const int expected_thread_priority = 102; const int actual_thread_priority = startEngineWithPriority(expected_thread_priority); // The `setpriority` system call doesn't define what happens when the thread priority is out of // range, and the behavior could be system dependent. On Linux, if the supplied priority value diff --git a/mobile/test/java/integration/AndroidEngineExplicitFlowTest.java b/mobile/test/java/integration/AndroidEngineExplicitFlowTest.java index e6e1986293..b29cbce256 100644 --- a/mobile/test/java/integration/AndroidEngineExplicitFlowTest.java +++ b/mobile/test/java/integration/AndroidEngineExplicitFlowTest.java @@ -481,8 +481,8 @@ public void post_multipleRequests_randomBehavior() throws Exception { mockWebServer.enqueue(new MockResponse().setBody("hello, world")); RequestScenario requestScenario = new RequestScenario() - .setHttpMethod(RequestMethod.GET) - .setUrl(mockWebServer.url("get/flowers").toString()) + .setHttpMethod(RequestMethod.POST) + .setUrl(mockWebServer.url("post/flowers").toString()) .addBody("This is my body part 1") .addBody("This is my body part 2") .setResponseBufferSize(20); // Larger than the response body size diff --git a/mobile/test/java/integration/BUILD b/mobile/test/java/integration/BUILD index 7154488b71..e987ab6456 100644 --- a/mobile/test/java/integration/BUILD +++ b/mobile/test/java/integration/BUILD @@ -51,6 +51,7 @@ envoy_mobile_android_test( srcs = [ "AndroidEngineExplicitFlowTest.java", ], + flaky = True, native_deps = [ "//test/jni:libenvoy_jni_with_test_extensions.so", ] + select({ diff --git a/mobile/test/java/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt b/mobile/test/java/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt index 276072825e..e5e621b205 100644 --- a/mobile/test/java/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt +++ b/mobile/test/java/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt @@ -92,7 +92,7 @@ class EnvoyConfigurationTest { quicCanonicalSuffixes: MutableList = mutableListOf(".opq.com", ".xyz.com"), enableGzipDecompression: Boolean = true, enableBrotliDecompression: Boolean = false, - enablePortMigration: Boolean = true, + numTimeoutsToTriggerPortMigration: Int = 4, enableSocketTagging: Boolean = false, enableInterfaceBinding: Boolean = false, h2ConnectionKeepaliveIdleIntervalMilliseconds: Int = 222, @@ -140,7 +140,7 @@ class EnvoyConfigurationTest { quicCanonicalSuffixes, enableGzipDecompression, enableBrotliDecompression, - enablePortMigration, + numTimeoutsToTriggerPortMigration, enableSocketTagging, enableInterfaceBinding, h2ConnectionKeepaliveIdleIntervalMilliseconds, diff --git a/mobile/test/java/org/chromium/net/BUILD b/mobile/test/java/org/chromium/net/BUILD index aefe1cb015..43b1efe7d7 100644 --- a/mobile/test/java/org/chromium/net/BUILD +++ b/mobile/test/java/org/chromium/net/BUILD @@ -305,6 +305,7 @@ envoy_mobile_android_test( srcs = [ "BidirectionalStreamTest.java", ], + flaky = True, native_deps = [ "//test/jni:libenvoy_jni_with_test_extensions.so", ] + select({ diff --git a/mobile/test/java/org/chromium/net/BidirectionalStreamTest.java b/mobile/test/java/org/chromium/net/BidirectionalStreamTest.java index 7f56f1c55b..53d53a23f9 100644 --- a/mobile/test/java/org/chromium/net/BidirectionalStreamTest.java +++ b/mobile/test/java/org/chromium/net/BidirectionalStreamTest.java @@ -78,9 +78,6 @@ public void setUp() throws Exception { @After public void tearDown() throws Exception { assertTrue(Http2TestServer.shutdownHttp2TestServer()); - if (mCronetEngine != null) { - mCronetEngine.shutdown(); - } } private static void checkResponseInfo(UrlResponseInfo responseInfo, String expectedUrl, diff --git a/source/common/config/null_grpc_mux_impl.h b/source/common/config/null_grpc_mux_impl.h index 453d723eb3..ce45640bfa 100644 --- a/source/common/config/null_grpc_mux_impl.h +++ b/source/common/config/null_grpc_mux_impl.h @@ -27,6 +27,12 @@ class NullGrpcMuxImpl : public GrpcMux, ENVOY_BUG(false, "unexpected request for on demand update"); } + absl::Status updateMuxSource(Grpc::RawAsyncClientPtr&&, Grpc::RawAsyncClientPtr&&, + CustomConfigValidatorsPtr&&, Stats::Scope&, BackOffStrategyPtr&&, + const envoy::config::core::v3::ApiConfigSource&) override { + return absl::UnimplementedError(""); + } + EdsResourcesCacheOptRef edsResourcesCache() override { return absl::nullopt; } void onWriteable() override {} diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc index 50a058aa79..65999f91b6 100644 --- a/source/common/config/utility.cc +++ b/source/common/config/utility.cc @@ -346,5 +346,28 @@ Utility::buildJitteredExponentialBackOffStrategy( default_base_interval_ms, default_base_interval_ms * 10, random); } +absl::Status Utility::validateTerminalFilters(const std::string& name, + const std::string& filter_type, + const std::string& filter_chain_type, + bool is_terminal_filter, + bool last_filter_in_current_config) { + if (is_terminal_filter && !last_filter_in_current_config) { + return absl::InvalidArgumentError( + fmt::format("Error: terminal filter named {} of type {} must be the " + "last filter in a {} filter chain.", + name, filter_type, filter_chain_type)); + } else if (!is_terminal_filter && last_filter_in_current_config) { + absl::string_view extra = ""; + if (filter_chain_type == "router upstream http") { + extra = " When upstream_http_filters are specified, they must explicitly end with an " + "UpstreamCodec filter."; + } + return absl::InvalidArgumentError(fmt::format("Error: non-terminal filter named {} of type " + "{} is the last filter in a {} filter chain.{}", + name, filter_type, filter_chain_type, extra)); + } + return absl::OkStatus(); +} + } // namespace Config } // namespace Envoy diff --git a/source/common/config/utility.h b/source/common/config/utility.h index ae9702984b..9467b753c7 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -424,19 +424,7 @@ class Utility { const std::string& filter_type, const std::string& filter_chain_type, bool is_terminal_filter, - bool last_filter_in_current_config) { - if (is_terminal_filter && !last_filter_in_current_config) { - return absl::InvalidArgumentError( - fmt::format("Error: terminal filter named {} of type {} must be the " - "last filter in a {} filter chain.", - name, filter_type, filter_chain_type)); - } else if (!is_terminal_filter && last_filter_in_current_config) { - return absl::InvalidArgumentError(fmt::format( - "Error: non-terminal filter named {} of type {} is the last filter in a {} filter chain.", - name, filter_type, filter_chain_type)); - } - return absl::OkStatus(); - } + bool last_filter_in_current_config); /** * Prepares the DNS failure refresh backoff strategy given the cluster configuration. diff --git a/source/common/formatter/substitution_formatter.h b/source/common/formatter/substitution_formatter.h index c2f1390388..f33f07471f 100644 --- a/source/common/formatter/substitution_formatter.h +++ b/source/common/formatter/substitution_formatter.h @@ -245,7 +245,7 @@ template class FormatterBaseImpl : public FormatterBase }; // Helper class to write value to output buffer in JSON style. -// NOTE: This helper class has duplicated logic with the Json::Streamer class but +// NOTE: This helper class has duplicated logic with the Json::BufferStreamer class but // provides lower level of APIs to operate on the output buffer (like control the // delimiters). This is designed for special scenario of substitution formatter and // is not intended to be used by other parts of the code. diff --git a/source/common/http/http3/conn_pool.cc b/source/common/http/http3/conn_pool.cc index 761a5048db..55963e30a0 100644 --- a/source/common/http/http3/conn_pool.cc +++ b/source/common/http/http3/conn_pool.cc @@ -115,7 +115,7 @@ Http3ConnPoolImpl::Http3ConnPoolImpl( random_generator, state, client_fn, codec_fn, protocol, {}, nullptr), quic_info_(dynamic_cast(quic_info)), server_id_(sni(transport_socket_options, host), - static_cast(host_->address()->ip()->port()), false), + static_cast(host_->address()->ip()->port())), connect_callback_(connect_callback), attempt_happy_eyeballs_(attempt_happy_eyeballs), network_observer_registry_(network_observer_registry) {} diff --git a/source/common/json/json_streamer.h b/source/common/json/json_streamer.h index 49239326e6..f63d1d439d 100644 --- a/source/common/json/json_streamer.h +++ b/source/common/json/json_streamer.h @@ -454,7 +454,7 @@ template class StreamerBase { /** * A Streamer that streams to a Buffer::Instance. */ -using Streamer = StreamerBase; +using BufferStreamer = StreamerBase; } // namespace Json } // namespace Envoy diff --git a/source/common/quic/envoy_quic_client_session.h b/source/common/quic/envoy_quic_client_session.h index 65e69027bc..8d3d1a8579 100644 --- a/source/common/quic/envoy_quic_client_session.h +++ b/source/common/quic/envoy_quic_client_session.h @@ -75,8 +75,7 @@ class EnvoyQuicClientSession : public QuicFilterManagerConnectionImpl, // PacketsToReadDelegate size_t numPacketsExpectedPerEventLoop() const override { - // Do one round of reading per active stream, or to see if there's a new - // active stream. + // Do one round of reading per active stream, or to see if there's a new active stream. return std::max(1, GetNumActiveStreams()) * Network::NUM_DATAGRAMS_PER_RECEIVE; } diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc index 88b1a96d76..cdfb83d255 100644 --- a/source/common/quic/envoy_quic_client_stream.cc +++ b/source/common/quic/envoy_quic_client_stream.cc @@ -258,7 +258,8 @@ bool EnvoyQuicClientStream::OnStopSending(quic::QuicResetStreamError error) { runResetCallbacks( quicRstErrorToEnvoyRemoteResetReason(error.internal_code()), Runtime::runtimeFeatureEnabled("envoy.reloadable_features.report_stream_reset_error_code") - ? quic::QuicRstStreamErrorCodeToString(error.internal_code()) + ? absl::StrCat(quic::QuicRstStreamErrorCodeToString(error.internal_code()), + "|FROM_PEER") : absl::string_view()); } return true; @@ -360,7 +361,7 @@ void EnvoyQuicClientStream::OnStreamReset(const quic::QuicRstStreamFrame& frame) runResetCallbacks( quicRstErrorToEnvoyRemoteResetReason(frame.error_code), Runtime::runtimeFeatureEnabled("envoy.reloadable_features.report_stream_reset_error_code") - ? quic::QuicRstStreamErrorCodeToString(frame.error_code) + ? absl::StrCat(quic::QuicRstStreamErrorCodeToString(frame.error_code), "|FROM_PEER") : absl::string_view()); } } @@ -374,7 +375,7 @@ void EnvoyQuicClientStream::ResetWithError(quic::QuicResetStreamError error) { runResetCallbacks( quicRstErrorToEnvoyLocalResetReason(error.internal_code()), Runtime::runtimeFeatureEnabled("envoy.reloadable_features.report_stream_reset_error_code") - ? quic::QuicRstStreamErrorCodeToString(error.internal_code()) + ? absl::StrCat(quic::QuicRstStreamErrorCodeToString(error.internal_code()), "|FROM_SELF") : absl::string_view()); if (session()->connection()->connected()) { quic::QuicSpdyClientStream::ResetWithError(error); diff --git a/source/common/quic/envoy_quic_server_stream.cc b/source/common/quic/envoy_quic_server_stream.cc index 331495df29..5da95f50d2 100644 --- a/source/common/quic/envoy_quic_server_stream.cc +++ b/source/common/quic/envoy_quic_server_stream.cc @@ -340,7 +340,8 @@ bool EnvoyQuicServerStream::OnStopSending(quic::QuicResetStreamError error) { runResetCallbacks( quicRstErrorToEnvoyRemoteResetReason(error.internal_code()), Runtime::runtimeFeatureEnabled("envoy.reloadable_features.report_stream_reset_error_code") - ? quic::QuicRstStreamErrorCodeToString(error.internal_code()) + ? absl::StrCat(quic::QuicRstStreamErrorCodeToString(error.internal_code()), + "|FROM_PEER") : absl::string_view()); } return true; @@ -360,7 +361,7 @@ void EnvoyQuicServerStream::OnStreamReset(const quic::QuicRstStreamFrame& frame) runResetCallbacks( quicRstErrorToEnvoyRemoteResetReason(frame.error_code), Runtime::runtimeFeatureEnabled("envoy.reloadable_features.report_stream_reset_error_code") - ? quic::QuicRstStreamErrorCodeToString(frame.error_code) + ? absl::StrCat(quic::QuicRstStreamErrorCodeToString(frame.error_code), "|FROM_PEER") : absl::string_view()); } } @@ -375,7 +376,8 @@ void EnvoyQuicServerStream::ResetWithError(quic::QuicResetStreamError error) { runResetCallbacks( quicRstErrorToEnvoyLocalResetReason(error.internal_code()), Runtime::runtimeFeatureEnabled("envoy.reloadable_features.report_stream_reset_error_code") - ? quic::QuicRstStreamErrorCodeToString(error.internal_code()) + ? absl::StrCat(quic::QuicRstStreamErrorCodeToString(error.internal_code()), + "|FROM_SELF") : absl::string_view()); } quic::QuicSpdyServerStreamBase::ResetWithError(error); diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 0b3ffdaefd..78b7c56537 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -478,7 +478,6 @@ class RetryPolicyImpl : public RetryPolicy { // that should be used when with this policy. std::vector> retry_host_predicate_configs_; - Upstream::RetryPrioritySharedPtr retry_priority_; // Name and config proto to use to create the RetryPriority to use with this policy. Default // initialized when no RetryPriority should be used. std::pair retry_priority_config_; diff --git a/source/common/router/scoped_config_impl.cc b/source/common/router/scoped_config_impl.cc index e6566a2f87..c400649efe 100644 --- a/source/common/router/scoped_config_impl.cc +++ b/source/common/router/scoped_config_impl.cc @@ -77,10 +77,10 @@ HeaderValueExtractorImpl::computeFragment(const Http::HeaderMap& headers) const return nullptr; } -ScopedRouteInfo::ScopedRouteInfo(envoy::config::route::v3::ScopedRouteConfiguration config_proto, +ScopedRouteInfo::ScopedRouteInfo(envoy::config::route::v3::ScopedRouteConfiguration&& config_proto, ConfigConstSharedPtr route_config) - : config_proto_(config_proto), route_config_(route_config), - config_hash_(MessageUtil::hash(config_proto)) { + : config_proto_(std::move(config_proto)), route_config_(route_config), + config_hash_(MessageUtil::hash(config_proto_)) { // TODO(stevenzzzz): Maybe worth a KeyBuilder abstraction when there are more than one type of // Fragment. for (const auto& fragment : config_proto_.key().fragments()) { diff --git a/source/common/router/scoped_config_impl.h b/source/common/router/scoped_config_impl.h index d7f8bd158f..ee134aaf66 100644 --- a/source/common/router/scoped_config_impl.h +++ b/source/common/router/scoped_config_impl.h @@ -77,7 +77,7 @@ class ScopeKeyBuilderImpl : public ScopeKeyBuilderBase { // ScopedRouteConfiguration and corresponding RouteConfigProvider. class ScopedRouteInfo { public: - ScopedRouteInfo(envoy::config::route::v3::ScopedRouteConfiguration config_proto, + ScopedRouteInfo(envoy::config::route::v3::ScopedRouteConfiguration&& config_proto, ConfigConstSharedPtr route_config); const ConfigConstSharedPtr& routeConfig() const { return route_config_; } @@ -89,9 +89,9 @@ class ScopedRouteInfo { uint64_t configHash() const { return config_hash_; } private: - envoy::config::route::v3::ScopedRouteConfiguration config_proto_; + const envoy::config::route::v3::ScopedRouteConfiguration config_proto_; ScopeKey scope_key_; - ConfigConstSharedPtr route_config_; + const ConfigConstSharedPtr route_config_; const uint64_t config_hash_; }; using ScopedRouteInfoConstSharedPtr = std::shared_ptr; diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index e459d0dc6b..b600825934 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -103,7 +103,7 @@ makeScopedRouteInfos(ProtobufTypes::ConstMessagePtrVector&& config_protos, config_provider_manager.routeConfigProviderManager().createStaticRouteConfigProvider( scoped_route_config.route_configuration(), factory_context, factory_context.messageValidationContext().staticValidationVisitor()); - scopes.push_back(std::make_shared(scoped_route_config, + scopes.push_back(std::make_shared(std::move(scoped_route_config), route_config_provider->configCast())); } diff --git a/source/extensions/config_subscription/grpc/grpc_mux_failover.h b/source/extensions/config_subscription/grpc/grpc_mux_failover.h index 19765df61f..1b7daa8b02 100644 --- a/source/extensions/config_subscription/grpc/grpc_mux_failover.h +++ b/source/extensions/config_subscription/grpc/grpc_mux_failover.h @@ -188,6 +188,8 @@ class GrpcMuxFailover : public GrpcStreamInterface, } private: + friend class GrpcMuxFailoverTest; + // A helper class that proxies the callbacks of GrpcStreamCallbacks for the primary service. class PrimaryGrpcStreamCallbacks : public GrpcStreamCallbacks { public: @@ -356,7 +358,15 @@ class GrpcMuxFailover : public GrpcStreamInterface, void onRemoteClose(Grpc::Status::GrpcStatus, const std::string&) override { PANIC("not implemented"); } - void closeStream() override { PANIC("not implemented"); } + void closeStream() override { + if (connectingToOrConnectedToPrimary()) { + ENVOY_LOG_MISC(debug, "Intentionally closing the primary gRPC stream"); + primary_grpc_stream_->closeStream(); + } else if (connectingToOrConnectedToFailover()) { + ENVOY_LOG_MISC(debug, "Intentionally closing the failover gRPC stream"); + failover_grpc_stream_->closeStream(); + } + } // The stream callbacks that will be invoked on the GrpcMux object, to notify // about the state of the underlying primary/failover stream. diff --git a/source/extensions/config_subscription/grpc/grpc_mux_impl.cc b/source/extensions/config_subscription/grpc/grpc_mux_impl.cc index 4317cdca1a..f036cea34e 100644 --- a/source/extensions/config_subscription/grpc/grpc_mux_impl.cc +++ b/source/extensions/config_subscription/grpc/grpc_mux_impl.cc @@ -60,14 +60,18 @@ std::string convertToWildcard(const std::string& resource_name) { } // namespace GrpcMuxImpl::GrpcMuxImpl(GrpcMuxContext& grpc_mux_context, bool skip_subsequent_node) - : grpc_stream_(createGrpcStreamObject(grpc_mux_context)), + : dispatcher_(grpc_mux_context.dispatcher_), + grpc_stream_(createGrpcStreamObject(std::move(grpc_mux_context.async_client_), + std::move(grpc_mux_context.failover_async_client_), + grpc_mux_context.service_method_, grpc_mux_context.scope_, + std::move(grpc_mux_context.backoff_strategy_), + grpc_mux_context.rate_limit_settings_)), local_info_(grpc_mux_context.local_info_), skip_subsequent_node_(skip_subsequent_node), config_validators_(std::move(grpc_mux_context.config_validators_)), xds_config_tracker_(grpc_mux_context.xds_config_tracker_), xds_resources_delegate_(grpc_mux_context.xds_resources_delegate_), eds_resources_cache_(std::move(grpc_mux_context.eds_resources_cache_)), target_xds_authority_(grpc_mux_context.target_xds_authority_), - dispatcher_(grpc_mux_context.dispatcher_), dynamic_update_callback_handle_( grpc_mux_context.local_info_.contextProvider().addDynamicContextUpdateCallback( [this](absl::string_view resource_type_url) { @@ -80,29 +84,33 @@ GrpcMuxImpl::GrpcMuxImpl(GrpcMuxContext& grpc_mux_context, bool skip_subsequent_ std::unique_ptr> -GrpcMuxImpl::createGrpcStreamObject(GrpcMuxContext& grpc_mux_context) { +GrpcMuxImpl::createGrpcStreamObject(Grpc::RawAsyncClientPtr&& async_client, + Grpc::RawAsyncClientPtr&& failover_async_client, + const Protobuf::MethodDescriptor& service_method, + Stats::Scope& scope, BackOffStrategyPtr&& backoff_strategy, + const RateLimitSettings& rate_limit_settings) { if (Runtime::runtimeFeatureEnabled("envoy.restart_features.xds_failover_support")) { return std::make_unique>( /*primary_stream_creator=*/ - [&grpc_mux_context]( + [&async_client, &service_method, &dispatcher = dispatcher_, &scope, &backoff_strategy, + &rate_limit_settings]( GrpcStreamCallbacks* callbacks) -> GrpcStreamInterfacePtr { return std::make_unique>( - callbacks, std::move(grpc_mux_context.async_client_), - grpc_mux_context.service_method_, grpc_mux_context.dispatcher_, - grpc_mux_context.scope_, std::move(grpc_mux_context.backoff_strategy_), - grpc_mux_context.rate_limit_settings_, + callbacks, std::move(async_client), service_method, dispatcher, scope, + std::move(backoff_strategy), rate_limit_settings, GrpcStream::ConnectedStateValue:: FIRST_ENTRY); }, /*failover_stream_creator=*/ - grpc_mux_context.failover_async_client_ + failover_async_client ? absl::make_optional( - [&grpc_mux_context]( + [&failover_async_client, &service_method, &dispatcher = dispatcher_, &scope, + &rate_limit_settings]( GrpcStreamCallbacks* callbacks) -> GrpcStreamInterfacePtr>( - callbacks, std::move(grpc_mux_context.failover_async_client_), - grpc_mux_context.service_method_, grpc_mux_context.dispatcher_, - grpc_mux_context.scope_, + callbacks, std::move(failover_async_client), service_method, dispatcher, + scope, // TODO(adisuissa): the backoff strategy for the failover should // be the same as the primary source. std::make_unique( GrpcMuxFailover:: DefaultFailoverBackoffMilliseconds), - grpc_mux_context.rate_limit_settings_, + rate_limit_settings, GrpcStream:: ConnectedStateValue::SECOND_ENTRY); }) : absl::nullopt, /*grpc_mux_callbacks=*/*this, - /*dispatch=*/grpc_mux_context.dispatcher_); + /*dispatch=*/dispatcher_); } return std::make_unique>( - this, std::move(grpc_mux_context.async_client_), grpc_mux_context.service_method_, - grpc_mux_context.dispatcher_, grpc_mux_context.scope_, - std::move(grpc_mux_context.backoff_strategy_), grpc_mux_context.rate_limit_settings_, + this, std::move(async_client), service_method, dispatcher_, scope, + std::move(backoff_strategy), rate_limit_settings, GrpcStream< envoy::service::discovery::v3::DiscoveryRequest, envoy::service::discovery::v3::DiscoveryResponse>::ConnectedStateValue::FIRST_ENTRY); @@ -292,6 +298,37 @@ GrpcMuxWatchPtr GrpcMuxImpl::addWatch(const std::string& type_url, return watch; } +absl::Status +GrpcMuxImpl::updateMuxSource(Grpc::RawAsyncClientPtr&& primary_async_client, + Grpc::RawAsyncClientPtr&& failover_async_client, + CustomConfigValidatorsPtr&& custom_config_validators, + Stats::Scope& scope, BackOffStrategyPtr&& backoff_strategy, + const envoy::config::core::v3::ApiConfigSource& ads_config_source) { + // Process the rate limit settings. + absl::StatusOr rate_limit_settings_or_error = + Utility::parseRateLimitSettings(ads_config_source); + RETURN_IF_NOT_OK_REF(rate_limit_settings_or_error.status()); + + const Protobuf::MethodDescriptor& service_method = + *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( + "envoy.service.discovery.v3.AggregatedDiscoveryService.StreamAggregatedResources"); + + // Disconnect from current xDS servers. + ENVOY_LOG_MISC(info, "Replacing xDS gRPC mux source"); + grpc_stream_->closeStream(); + grpc_stream_ = createGrpcStreamObject(std::move(primary_async_client), + std::move(failover_async_client), service_method, scope, + std::move(backoff_strategy), *rate_limit_settings_or_error); + + // Update the config validators. + config_validators_ = std::move(custom_config_validators); + + // Start the subscriptions over the grpc_stream. + grpc_stream_->establishNewStream(); + + return absl::OkStatus(); +} + ScopedResume GrpcMuxImpl::pause(const std::string& type_url) { return pause(std::vector{type_url}); } diff --git a/source/extensions/config_subscription/grpc/grpc_mux_impl.h b/source/extensions/config_subscription/grpc/grpc_mux_impl.h index 885942f3fb..c53d51cd09 100644 --- a/source/extensions/config_subscription/grpc/grpc_mux_impl.h +++ b/source/extensions/config_subscription/grpc/grpc_mux_impl.h @@ -73,6 +73,13 @@ class GrpcMuxImpl : public GrpcMux, return makeOptRefFromPtr(eds_resources_cache_.get()); } + absl::Status + updateMuxSource(Grpc::RawAsyncClientPtr&& primary_async_client, + Grpc::RawAsyncClientPtr&& failover_async_client, + CustomConfigValidatorsPtr&& custom_config_validators, Stats::Scope& scope, + BackOffStrategyPtr&& backoff_strategy, + const envoy::config::core::v3::ApiConfigSource& ads_config_source) override; + void handleDiscoveryResponse( std::unique_ptr&& message); @@ -100,11 +107,13 @@ class GrpcMuxImpl : public GrpcMux, private: // Helper function to create the grpc_stream_ object. - // TODO(adisuissa): this should be removed when envoy.restart_features.xds_failover_support - // is deprecated. std::unique_ptr> - createGrpcStreamObject(GrpcMuxContext& grpc_mux_context); + createGrpcStreamObject(Grpc::RawAsyncClientPtr&& async_client, + Grpc::RawAsyncClientPtr&& failover_async_client, + const Protobuf::MethodDescriptor& service_method, Stats::Scope& scope, + BackOffStrategyPtr&& backoff_strategy, + const RateLimitSettings& rate_limit_settings); void drainRequests(); void setRetryTimer(); @@ -272,6 +281,7 @@ class GrpcMuxImpl : public GrpcMux, ApiState& api_state, const std::string& type_url, const std::string& version_info, bool call_delegate); + Event::Dispatcher& dispatcher_; // Multiplexes the stream to the primary and failover sources. // TODO(adisuissa): Once envoy.restart_features.xds_failover_support is deprecated, // convert from unique_ptr to GrpcMuxFailover directly. @@ -301,7 +311,6 @@ class GrpcMuxImpl : public GrpcMux, // URL. std::unique_ptr> request_queue_; - Event::Dispatcher& dispatcher_; Common::CallbackHandlePtr dynamic_update_callback_handle_; bool started_{false}; diff --git a/source/extensions/config_subscription/grpc/new_grpc_mux_impl.cc b/source/extensions/config_subscription/grpc/new_grpc_mux_impl.cc index 1a47a63572..6315853829 100644 --- a/source/extensions/config_subscription/grpc/new_grpc_mux_impl.cc +++ b/source/extensions/config_subscription/grpc/new_grpc_mux_impl.cc @@ -36,7 +36,12 @@ using AllMuxes = ThreadSafeSingleton; } // namespace NewGrpcMuxImpl::NewGrpcMuxImpl(GrpcMuxContext& grpc_mux_context) - : grpc_stream_(createGrpcStreamObject(grpc_mux_context)), + : dispatcher_(grpc_mux_context.dispatcher_), + grpc_stream_(createGrpcStreamObject(std::move(grpc_mux_context.async_client_), + std::move(grpc_mux_context.failover_async_client_), + grpc_mux_context.service_method_, grpc_mux_context.scope_, + std::move(grpc_mux_context.backoff_strategy_), + grpc_mux_context.rate_limit_settings_)), local_info_(grpc_mux_context.local_info_), config_validators_(std::move(grpc_mux_context.config_validators_)), dynamic_update_callback_handle_( @@ -45,7 +50,6 @@ NewGrpcMuxImpl::NewGrpcMuxImpl(GrpcMuxContext& grpc_mux_context) onDynamicContextUpdate(resource_type_url); return absl::OkStatus(); })), - dispatcher_(grpc_mux_context.dispatcher_), xds_config_tracker_(grpc_mux_context.xds_config_tracker_), eds_resources_cache_(std::move(grpc_mux_context.eds_resources_cache_)) { AllMuxes::get().insert(this); @@ -53,30 +57,34 @@ NewGrpcMuxImpl::NewGrpcMuxImpl(GrpcMuxContext& grpc_mux_context) std::unique_ptr> -NewGrpcMuxImpl::createGrpcStreamObject(GrpcMuxContext& grpc_mux_context) { +NewGrpcMuxImpl::createGrpcStreamObject(Grpc::RawAsyncClientPtr&& async_client, + Grpc::RawAsyncClientPtr&& failover_async_client, + const Protobuf::MethodDescriptor& service_method, + Stats::Scope& scope, BackOffStrategyPtr&& backoff_strategy, + const RateLimitSettings& rate_limit_settings) { if (Runtime::runtimeFeatureEnabled("envoy.restart_features.xds_failover_support")) { return std::make_unique>( /*primary_stream_creator=*/ - [&grpc_mux_context]( + [&async_client, &service_method, &dispatcher = dispatcher_, &scope, &backoff_strategy, + &rate_limit_settings]( GrpcStreamCallbacks* callbacks) -> GrpcStreamInterfacePtr { return std::make_unique< GrpcStream>( - callbacks, std::move(grpc_mux_context.async_client_), - grpc_mux_context.service_method_, grpc_mux_context.dispatcher_, - grpc_mux_context.scope_, std::move(grpc_mux_context.backoff_strategy_), - grpc_mux_context.rate_limit_settings_, + callbacks, std::move(async_client), service_method, dispatcher, scope, + std::move(backoff_strategy), rate_limit_settings, GrpcStream:: ConnectedStateValue::FIRST_ENTRY); }, /*failover_stream_creator=*/ - grpc_mux_context.failover_async_client_ + failover_async_client ? absl::make_optional( - [&grpc_mux_context]( + [&failover_async_client, &service_method, &dispatcher = dispatcher_, &scope, + &rate_limit_settings]( GrpcStreamCallbacks* callbacks) -> GrpcStreamInterfacePtr< @@ -85,29 +93,27 @@ NewGrpcMuxImpl::createGrpcStreamObject(GrpcMuxContext& grpc_mux_context) { return std::make_unique< GrpcStream>( - callbacks, std::move(grpc_mux_context.failover_async_client_), - grpc_mux_context.service_method_, grpc_mux_context.dispatcher_, - grpc_mux_context.scope_, + callbacks, std::move(failover_async_client), service_method, dispatcher, + scope, // TODO(adisuissa): the backoff strategy for the failover should // be the same as the primary source. std::make_unique( GrpcMuxFailover:: DefaultFailoverBackoffMilliseconds), - grpc_mux_context.rate_limit_settings_, + rate_limit_settings, GrpcStream:: ConnectedStateValue::SECOND_ENTRY); }) : absl::nullopt, /*grpc_mux_callbacks=*/*this, - /*dispatch=*/grpc_mux_context.dispatcher_); + /*dispatch=*/dispatcher_); } return std::make_unique>( - this, std::move(grpc_mux_context.async_client_), grpc_mux_context.service_method_, - grpc_mux_context.dispatcher_, grpc_mux_context.scope_, - std::move(grpc_mux_context.backoff_strategy_), grpc_mux_context.rate_limit_settings_, + this, std::move(async_client), service_method, dispatcher_, scope, + std::move(backoff_strategy), rate_limit_settings, GrpcStream< envoy::service::discovery::v3::DeltaDiscoveryRequest, envoy::service::discovery::v3::DeltaDiscoveryResponse>::ConnectedStateValue::FIRST_ENTRY); @@ -246,6 +252,41 @@ GrpcMuxWatchPtr NewGrpcMuxImpl::addWatch(const std::string& type_url, return std::make_unique(type_url, watch, *this, options); } +absl::Status +NewGrpcMuxImpl::updateMuxSource(Grpc::RawAsyncClientPtr&& primary_async_client, + Grpc::RawAsyncClientPtr&& failover_async_client, + CustomConfigValidatorsPtr&& custom_config_validators, + Stats::Scope& scope, BackOffStrategyPtr&& backoff_strategy, + const envoy::config::core::v3::ApiConfigSource& ads_config_source) { + // Process the rate limit settings. + absl::StatusOr rate_limit_settings_or_error = + Utility::parseRateLimitSettings(ads_config_source); + RETURN_IF_NOT_OK_REF(rate_limit_settings_or_error.status()); + + const Protobuf::MethodDescriptor& service_method = + *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( + "envoy.service.discovery.v3.AggregatedDiscoveryService.DeltaAggregatedResources"); + + // Disconnect from current xDS servers. + ENVOY_LOG_MISC(info, "Replacing the xDS gRPC mux source"); + grpc_stream_->closeStream(); + grpc_stream_ = createGrpcStreamObject(std::move(primary_async_client), + std::move(failover_async_client), service_method, scope, + std::move(backoff_strategy), *rate_limit_settings_or_error); + + // Update the config validators. + config_validators_ = std::move(custom_config_validators); + // Update the watch map's config validators. + for (auto& [type_url, subscription] : subscriptions_) { + subscription->watch_map_.setConfigValidators(config_validators_.get()); + } + + // Start the subscriptions over the grpc_stream. + grpc_stream_->establishNewStream(); + + return absl::OkStatus(); +} + // Updates the list of resource names watched by the given watch. If an added name is new across // the whole subscription, or if a removed name has no other watch interested in it, then the // subscription will enqueue and attempt to send an appropriate discovery request. @@ -320,7 +361,7 @@ void NewGrpcMuxImpl::addSubscription(const std::string& type_url, } subscriptions_.emplace( type_url, std::make_unique(type_url, local_info_, use_namespace_matching, - dispatcher_, *config_validators_.get(), + dispatcher_, config_validators_.get(), xds_config_tracker_, resources_cache)); subscription_ordering_.emplace_back(type_url); } diff --git a/source/extensions/config_subscription/grpc/new_grpc_mux_impl.h b/source/extensions/config_subscription/grpc/new_grpc_mux_impl.h index 741ea6856e..5c35dd6e17 100644 --- a/source/extensions/config_subscription/grpc/new_grpc_mux_impl.h +++ b/source/extensions/config_subscription/grpc/new_grpc_mux_impl.h @@ -78,6 +78,13 @@ class NewGrpcMuxImpl // TODO(fredlas) remove this from the GrpcMux interface. void start() override; + absl::Status + updateMuxSource(Grpc::RawAsyncClientPtr&& primary_async_client, + Grpc::RawAsyncClientPtr&& failover_async_client, + CustomConfigValidatorsPtr&& custom_config_validators, Stats::Scope& scope, + BackOffStrategyPtr&& backoff_strategy, + const envoy::config::core::v3::ApiConfigSource& ads_config_source) override; + GrpcStreamInterface& grpcStreamForTest() { @@ -95,7 +102,7 @@ class NewGrpcMuxImpl struct SubscriptionStuff { SubscriptionStuff(const std::string& type_url, const LocalInfo::LocalInfo& local_info, const bool use_namespace_matching, Event::Dispatcher& dispatcher, - CustomConfigValidators& config_validators, + CustomConfigValidators* config_validators, XdsConfigTrackerOptRef xds_config_tracker, EdsResourcesCacheOptRef eds_resources_cache) : watch_map_(use_namespace_matching, type_url, config_validators, eds_resources_cache), @@ -149,11 +156,13 @@ class NewGrpcMuxImpl }; // Helper function to create the grpc_stream_ object. - // TODO(adisuissa): this should be removed when envoy.restart_features.xds_failover_support - // is deprecated. std::unique_ptr> - createGrpcStreamObject(GrpcMuxContext& grpc_mux_context); + createGrpcStreamObject(Grpc::RawAsyncClientPtr&& async_client, + Grpc::RawAsyncClientPtr&& failover_async_client, + const Protobuf::MethodDescriptor& service_method, Stats::Scope& scope, + BackOffStrategyPtr&& backoff_strategy, + const RateLimitSettings& rate_limit_settings); void removeWatch(const std::string& type_url, Watch* watch); @@ -196,6 +205,7 @@ class NewGrpcMuxImpl // the order of Envoy's dependency ordering). std::list subscription_ordering_; + Event::Dispatcher& dispatcher_; // Multiplexes the stream to the primary and failover sources. // TODO(adisuissa): Once envoy.restart_features.xds_failover_support is deprecated, // convert from unique_ptr to GrpcMuxFailover directly. @@ -206,7 +216,6 @@ class NewGrpcMuxImpl const LocalInfo::LocalInfo& local_info_; CustomConfigValidatorsPtr config_validators_; Common::CallbackHandlePtr dynamic_update_callback_handle_; - Event::Dispatcher& dispatcher_; XdsConfigTrackerOptRef xds_config_tracker_; EdsResourcesCachePtr eds_resources_cache_; diff --git a/source/extensions/config_subscription/grpc/watch_map.cc b/source/extensions/config_subscription/grpc/watch_map.cc index 815bcca485..ffa57508ee 100644 --- a/source/extensions/config_subscription/grpc/watch_map.cc +++ b/source/extensions/config_subscription/grpc/watch_map.cc @@ -154,7 +154,7 @@ void WatchMap::onConfigUpdate(const std::vector& resources, } // Execute external config validators. - config_validators_.executeValidators(type_url_, resources); + config_validators_->executeValidators(type_url_, resources); const bool map_is_single_wildcard = (watches_.size() == 1 && wildcard_watches_.size() == 1); // We just bundled up the updates into nice per-watch packages. Now, deliver them. @@ -254,7 +254,7 @@ void WatchMap::onConfigUpdate( } // Execute external config validators. - config_validators_.executeValidators(type_url_, decoded_resources, removed_resources); + config_validators_->executeValidators(type_url_, decoded_resources, removed_resources); // We just bundled up the updates into nice per-watch packages. Now, deliver them. for (const auto& [cur_watch, resource_to_add] : per_watch_added) { diff --git a/source/extensions/config_subscription/grpc/watch_map.h b/source/extensions/config_subscription/grpc/watch_map.h index 47cd43b458..07140804ed 100644 --- a/source/extensions/config_subscription/grpc/watch_map.h +++ b/source/extensions/config_subscription/grpc/watch_map.h @@ -73,7 +73,7 @@ struct Watch { class WatchMap : public UntypedConfigUpdateCallbacks, public Logger::Loggable { public: WatchMap(const bool use_namespace_matching, const std::string& type_url, - CustomConfigValidators& config_validators, EdsResourcesCacheOptRef eds_resources_cache) + CustomConfigValidators* config_validators, EdsResourcesCacheOptRef eds_resources_cache) : use_namespace_matching_(use_namespace_matching), type_url_(type_url), config_validators_(config_validators), eds_resources_cache_(eds_resources_cache) { // If eds resources cache is provided, then the type must be ClusterLoadAssignment. @@ -114,6 +114,10 @@ class WatchMap : public UntypedConfigUpdateCallbacks, public Logger::Loggable; template GrpcMuxImpl::GrpcMuxImpl(std::unique_ptr subscription_state_factory, GrpcMuxContext& grpc_mux_context, bool skip_subsequent_node) - : grpc_stream_(createGrpcStreamObject(grpc_mux_context)), + : dispatcher_(grpc_mux_context.dispatcher_), + grpc_stream_(createGrpcStreamObject(std::move(grpc_mux_context.async_client_), + std::move(grpc_mux_context.failover_async_client_), + grpc_mux_context.service_method_, grpc_mux_context.scope_, + std::move(grpc_mux_context.backoff_strategy_), + grpc_mux_context.rate_limit_settings_)), subscription_state_factory_(std::move(subscription_state_factory)), skip_subsequent_node_(skip_subsequent_node), local_info_(grpc_mux_context.local_info_), dynamic_update_callback_handle_( @@ -59,44 +64,46 @@ GrpcMuxImpl::GrpcMuxImpl(std::unique_ptr subscription_state_fac } template -std::unique_ptr> -GrpcMuxImpl::createGrpcStreamObject(GrpcMuxContext& grpc_mux_context) { +std::unique_ptr> GrpcMuxImpl::createGrpcStreamObject( + Grpc::RawAsyncClientPtr&& async_client, Grpc::RawAsyncClientPtr&& failover_async_client, + const Protobuf::MethodDescriptor& service_method, Stats::Scope& scope, + BackOffStrategyPtr&& backoff_strategy, const RateLimitSettings& rate_limit_settings) { if (Runtime::runtimeFeatureEnabled("envoy.restart_features.xds_failover_support")) { return std::make_unique>( /*primary_stream_creator=*/ - [&grpc_mux_context](GrpcStreamCallbacks* callbacks) -> GrpcStreamInterfacePtr { + [&async_client, &service_method, &dispatcher = dispatcher_, &scope, &backoff_strategy, + &rate_limit_settings]( + GrpcStreamCallbacks* callbacks) -> GrpcStreamInterfacePtr { return std::make_unique>( - callbacks, std::move(grpc_mux_context.async_client_), - grpc_mux_context.service_method_, grpc_mux_context.dispatcher_, - grpc_mux_context.scope_, std::move(grpc_mux_context.backoff_strategy_), - grpc_mux_context.rate_limit_settings_, + callbacks, std::move(async_client), service_method, dispatcher, scope, + std::move(backoff_strategy), rate_limit_settings, GrpcStream::ConnectedStateValue::FIRST_ENTRY); }, /*failover_stream_creator=*/ - grpc_mux_context.failover_async_client_ - ? absl::make_optional([&grpc_mux_context](GrpcStreamCallbacks* callbacks) - -> GrpcStreamInterfacePtr { - return std::make_unique>( - callbacks, std::move(grpc_mux_context.failover_async_client_), - grpc_mux_context.service_method_, grpc_mux_context.dispatcher_, - grpc_mux_context.scope_, - // TODO(adisuissa): the backoff strategy for the failover should - // be the same as the primary source. - std::make_unique( - GrpcMuxFailover::DefaultFailoverBackoffMilliseconds), - grpc_mux_context.rate_limit_settings_, - GrpcStream::ConnectedStateValue::SECOND_ENTRY); - }) + failover_async_client + ? absl::make_optional( + [&failover_async_client, &service_method, &dispatcher = dispatcher_, &scope, + &rate_limit_settings]( + GrpcStreamCallbacks* callbacks) -> GrpcStreamInterfacePtr { + return std::make_unique>( + callbacks, std::move(failover_async_client), service_method, dispatcher, + scope, + // TODO(adisuissa): the backoff strategy for the failover should + // be the same as the primary source. + std::make_unique( + GrpcMuxFailover::DefaultFailoverBackoffMilliseconds), + rate_limit_settings, GrpcStream::ConnectedStateValue::SECOND_ENTRY); + }) : absl::nullopt, /*grpc_mux_callbacks=*/*this, - /*dispatch=*/grpc_mux_context.dispatcher_); + /*dispatch=*/dispatcher_); } - return std::make_unique>( - this, std::move(grpc_mux_context.async_client_), grpc_mux_context.service_method_, - grpc_mux_context.dispatcher_, grpc_mux_context.scope_, - std::move(grpc_mux_context.backoff_strategy_), grpc_mux_context.rate_limit_settings_, - GrpcStream::ConnectedStateValue::FIRST_ENTRY); + return std::make_unique>(this, std::move(async_client), service_method, + dispatcher_, scope, std::move(backoff_strategy), + rate_limit_settings, + GrpcStream::ConnectedStateValue::FIRST_ENTRY); } + template GrpcMuxImpl::~GrpcMuxImpl() { AllMuxes::get().erase(this); } @@ -134,7 +141,7 @@ Config::GrpcMuxWatchPtr GrpcMuxImpl::addWatch( watch_map = watch_maps_ .emplace(type_url, std::make_unique(options.use_namespace_matching_, type_url, - *config_validators_.get(), resources_cache)) + config_validators_.get(), resources_cache)) .first; subscriptions_.emplace(type_url, subscription_state_factory_->makeSubscriptionState( type_url, *watch_maps_[type_url], resource_decoder, @@ -221,6 +228,40 @@ ScopedResume GrpcMuxImpl::pause(const std::vector typ }); } +template +absl::Status GrpcMuxImpl::updateMuxSource( + Grpc::RawAsyncClientPtr&& primary_async_client, Grpc::RawAsyncClientPtr&& failover_async_client, + CustomConfigValidatorsPtr&& custom_config_validators, Stats::Scope& scope, + BackOffStrategyPtr&& backoff_strategy, + const envoy::config::core::v3::ApiConfigSource& ads_config_source) { + // Process the rate limit settings. + absl::StatusOr rate_limit_settings_or_error = + Utility::parseRateLimitSettings(ads_config_source); + RETURN_IF_NOT_OK_REF(rate_limit_settings_or_error.status()); + + const Protobuf::MethodDescriptor& service_method = + *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(methodName()); + + // Disconnect from current xDS servers. + ENVOY_LOG_MISC(info, "Replacing the xDS gRPC mux source"); + grpc_stream_->closeStream(); + grpc_stream_ = createGrpcStreamObject(std::move(primary_async_client), + std::move(failover_async_client), service_method, scope, + std::move(backoff_strategy), *rate_limit_settings_or_error); + + // Update the config validators. + config_validators_ = std::move(custom_config_validators); + // Update the watch map's config validators. + for (auto& [type_url, watch_map] : watch_maps_) { + watch_map->setConfigValidators(config_validators_.get()); + } + + // Start the subscriptions over the grpc_stream. + grpc_stream_->establishNewStream(); + + return absl::OkStatus(); +} + template void GrpcMuxImpl::sendGrpcMessage(RQ& msg_proto, S& sub_state) { if (sub_state.dynamicContextChanged() || !anyRequestSentYetInCurrentStream() || diff --git a/source/extensions/config_subscription/grpc/xds_mux/grpc_mux_impl.h b/source/extensions/config_subscription/grpc/xds_mux/grpc_mux_impl.h index 37f0c31f17..4639735140 100644 --- a/source/extensions/config_subscription/grpc/xds_mux/grpc_mux_impl.h +++ b/source/extensions/config_subscription/grpc/xds_mux/grpc_mux_impl.h @@ -105,6 +105,13 @@ class GrpcMuxImpl : public GrpcStreamCallbacks, genericHandleResponse(message->type_url(), *message, control_plane_stats); } + absl::Status + updateMuxSource(Grpc::RawAsyncClientPtr&& primary_async_client, + Grpc::RawAsyncClientPtr&& failover_async_client, + CustomConfigValidatorsPtr&& custom_config_validators, Stats::Scope& scope, + BackOffStrategyPtr&& backoff_strategy, + const envoy::config::core::v3::ApiConfigSource& ads_config_source) override; + EdsResourcesCacheOptRef edsResourcesCache() override { return makeOptRefFromPtr(eds_resources_cache_.get()); } @@ -165,12 +172,16 @@ class GrpcMuxImpl : public GrpcStreamCallbacks, } const LocalInfo::LocalInfo& localInfo() const { return local_info_; } + virtual absl::string_view methodName() const PURE; + private: // Helper function to create the grpc_stream_ object. // TODO(adisuissa): this should be removed when envoy.restart_features.xds_failover_support // is deprecated. - std::unique_ptr> - createGrpcStreamObject(GrpcMuxContext& grpc_mux_context); + std::unique_ptr> createGrpcStreamObject( + Grpc::RawAsyncClientPtr&& async_client, Grpc::RawAsyncClientPtr&& failover_async_client, + const Protobuf::MethodDescriptor& service_method, Stats::Scope& scope, + BackOffStrategyPtr&& backoff_strategy, const RateLimitSettings& rate_limit_settings); // Checks whether external conditions allow sending a DeltaDiscoveryRequest. (Does not check // whether we *want* to send a (Delta)DiscoveryRequest). @@ -187,6 +198,7 @@ class GrpcMuxImpl : public GrpcStreamCallbacks, // Invoked when dynamic context parameters change for a resource type. void onDynamicContextUpdate(absl::string_view resource_type_url); + Event::Dispatcher& dispatcher_; // Multiplexes the stream to the primary and failover sources. // TODO(adisuissa): Once envoy.restart_features.xds_failover_support is deprecated, // convert from unique_ptr to GrpcMuxFailover directly. @@ -248,6 +260,11 @@ class GrpcMuxDelta : public GrpcMuxImpl& for_update) override; + +private: + absl::string_view methodName() const override { + return "envoy.service.discovery.v3.AggregatedDiscoveryService.DeltaAggregatedResources"; + } }; class GrpcMuxSotw : public GrpcMuxImpl&) override { ENVOY_BUG(false, "unexpected request for on demand update"); } + +private: + absl::string_view methodName() const override { + return "envoy.service.discovery.v3.AggregatedDiscoveryService.StreamAggregatedResources"; + } }; class NullGrpcMuxImpl : public GrpcMux { @@ -277,6 +299,12 @@ class NullGrpcMuxImpl : public GrpcMux { SubscriptionCallbacks&, OpaqueResourceDecoderSharedPtr, const SubscriptionOptions&) override; + absl::Status updateMuxSource(Grpc::RawAsyncClientPtr&&, Grpc::RawAsyncClientPtr&&, + CustomConfigValidatorsPtr&&, Stats::Scope&, BackOffStrategyPtr&&, + const envoy::config::core::v3::ApiConfigSource&) override { + return absl::UnimplementedError(""); + } + void requestOnDemandUpdate(const std::string&, const absl::flat_hash_set&) override { ENVOY_BUG(false, "unexpected request for on demand update"); } diff --git a/source/extensions/filters/http/ext_proc/BUILD b/source/extensions/filters/http/ext_proc/BUILD index c6cb55ffb2..2fafe8df47 100644 --- a/source/extensions/filters/http/ext_proc/BUILD +++ b/source/extensions/filters/http/ext_proc/BUILD @@ -9,6 +9,15 @@ licenses(["notice"]) # Apache 2 envoy_extension_package() +envoy_cc_library( + name = "client_base_interface", + hdrs = ["client_base.h"], + tags = ["skip_on_windows"], + deps = [ + "@envoy_api//envoy/service/ext_proc/v3:pkg_cc_proto", + ], +) + envoy_cc_library( name = "ext_proc", srcs = [ @@ -21,6 +30,7 @@ envoy_cc_library( ], tags = ["skip_on_windows"], deps = [ + ":client_base_interface", ":client_interface", ":matching_utils_lib", ":mutation_utils_lib", @@ -34,6 +44,7 @@ envoy_cc_library( "//source/common/runtime:runtime_features_lib", "//source/extensions/filters/common/mutation_rules:mutation_rules_lib", "//source/extensions/filters/http/common:pass_through_filter_lib", + "//source/extensions/filters/http/ext_proc/http_client:http_client_lib", "@com_google_absl//absl/status", "@com_google_absl//absl/strings:str_format", "@com_google_absl//absl/strings:string_view", @@ -53,6 +64,7 @@ envoy_cc_extension( ":client_lib", ":ext_proc", "//source/extensions/filters/http/common:factory_base_lib", + "//source/extensions/filters/http/ext_proc/http_client:http_client_lib", "@envoy_api//envoy/extensions/filters/http/ext_proc/v3:pkg_cc_proto", ], ) @@ -62,6 +74,7 @@ envoy_cc_library( hdrs = ["client.h"], tags = ["skip_on_windows"], deps = [ + ":client_base_interface", "//envoy/grpc:async_client_manager_interface", "//envoy/grpc:status", "//envoy/stream_info:stream_info_interface", diff --git a/source/extensions/filters/http/ext_proc/client.h b/source/extensions/filters/http/ext_proc/client.h index 413bbcac77..d60f417051 100644 --- a/source/extensions/filters/http/ext_proc/client.h +++ b/source/extensions/filters/http/ext_proc/client.h @@ -10,13 +10,14 @@ #include "envoy/stream_info/stream_info.h" #include "source/common/http/sidestream_watermark.h" +#include "source/extensions/filters/http/ext_proc/client_base.h" namespace Envoy { namespace Extensions { namespace HttpFilters { namespace ExternalProcessing { -class ExternalProcessorStream { +class ExternalProcessorStream : public StreamBase { public: virtual ~ExternalProcessorStream() = default; virtual void send(envoy::service::ext_proc::v3::ProcessingRequest&& request, @@ -30,7 +31,7 @@ class ExternalProcessorStream { using ExternalProcessorStreamPtr = std::unique_ptr; -class ExternalProcessorCallbacks { +class ExternalProcessorCallbacks : public RequestCallbacks { public: virtual ~ExternalProcessorCallbacks() = default; virtual void onReceiveMessage( @@ -40,7 +41,7 @@ class ExternalProcessorCallbacks { virtual void logGrpcStreamInfo() PURE; }; -class ExternalProcessorClient { +class ExternalProcessorClient : public ClientBase { public: virtual ~ExternalProcessorClient() = default; virtual ExternalProcessorStreamPtr @@ -48,8 +49,6 @@ class ExternalProcessorClient { const Grpc::GrpcServiceConfigWithHashKey& config_with_hash_key, const Http::AsyncClient::StreamOptions& options, Http::StreamFilterSidestreamWatermarkCallbacks& sidestream_watermark_callbacks) PURE; - virtual ExternalProcessorStream* stream() PURE; - virtual void setStream(ExternalProcessorStream* stream) PURE; }; using ExternalProcessorClientPtr = std::unique_ptr; diff --git a/source/extensions/filters/http/ext_proc/client_base.h b/source/extensions/filters/http/ext_proc/client_base.h new file mode 100644 index 0000000000..d37fd0c1f5 --- /dev/null +++ b/source/extensions/filters/http/ext_proc/client_base.h @@ -0,0 +1,47 @@ +#pragma once + +#include + +#include "envoy/service/ext_proc/v3/external_processor.pb.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace ExternalProcessing { + +/** + * Async callbacks used during external processing. + */ +class RequestCallbacks { +public: + virtual ~RequestCallbacks() = default; + virtual void onComplete(envoy::service::ext_proc::v3::ProcessingResponse& response) PURE; + virtual void onError() PURE; +}; + +/** + * Stream base class used during external processing. + */ +class StreamBase { +public: + virtual ~StreamBase() = default; +}; + +/** + * Async client base class used during external processing. + */ +class ClientBase { +public: + virtual ~ClientBase() = default; + virtual void sendRequest(envoy::service::ext_proc::v3::ProcessingRequest&& request, + bool end_stream, const uint64_t stream_id, RequestCallbacks* callbacks, + StreamBase* stream) PURE; + virtual void cancel() PURE; +}; + +using ClientBasePtr = std::unique_ptr; + +} // namespace ExternalProcessing +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/ext_proc/client_impl.cc b/source/extensions/filters/http/ext_proc/client_impl.cc index fef44968dc..b52c919699 100644 --- a/source/extensions/filters/http/ext_proc/client_impl.cc +++ b/source/extensions/filters/http/ext_proc/client_impl.cc @@ -24,6 +24,13 @@ ExternalProcessorStreamPtr ExternalProcessorClientImpl::start( sidestream_watermark_callbacks); } +void ExternalProcessorClientImpl::sendRequest( + envoy::service::ext_proc::v3::ProcessingRequest&& request, bool end_stream, const uint64_t, + RequestCallbacks*, StreamBase* stream) { + ExternalProcessorStream* grpc_stream = dynamic_cast(stream); + grpc_stream->send(std::move(request), end_stream); +} + ExternalProcessorStreamPtr ExternalProcessorStreamImpl::create( Grpc::AsyncClient&& client, ExternalProcessorCallbacks& callbacks, const Http::AsyncClient::StreamOptions& options, diff --git a/source/extensions/filters/http/ext_proc/client_impl.h b/source/extensions/filters/http/ext_proc/client_impl.h index 8ef177cda0..d4ec819c73 100644 --- a/source/extensions/filters/http/ext_proc/client_impl.h +++ b/source/extensions/filters/http/ext_proc/client_impl.h @@ -32,15 +32,14 @@ class ExternalProcessorClientImpl : public ExternalProcessorClient { const Grpc::GrpcServiceConfigWithHashKey& config_with_hash_key, const Http::AsyncClient::StreamOptions& options, Http::StreamFilterSidestreamWatermarkCallbacks& sidestream_watermark_callbacks) override; - ExternalProcessorStream* stream() override { return stream_; } - void setStream(ExternalProcessorStream* stream) override { stream_ = stream; } + void sendRequest(envoy::service::ext_proc::v3::ProcessingRequest&& request, bool end_stream, + const uint64_t stream_id, RequestCallbacks* callbacks, + StreamBase* stream) override; + void cancel() override {} private: Grpc::AsyncClientManager& client_manager_; Stats::Scope& scope_; - // The gRPC stream to the external processor, which will be opened - // when it's time to send the first message. - ExternalProcessorStream* stream_ = nullptr; }; class ExternalProcessorStreamImpl : public ExternalProcessorStream, diff --git a/source/extensions/filters/http/ext_proc/config.cc b/source/extensions/filters/http/ext_proc/config.cc index 134fa8d190..076325b2d7 100644 --- a/source/extensions/filters/http/ext_proc/config.cc +++ b/source/extensions/filters/http/ext_proc/config.cc @@ -3,6 +3,7 @@ #include "source/extensions/filters/common/expr/evaluator.h" #include "source/extensions/filters/http/ext_proc/client_impl.h" #include "source/extensions/filters/http/ext_proc/ext_proc.h" +#include "source/extensions/filters/http/ext_proc/http_client/http_client_impl.h" namespace Envoy { namespace Extensions { @@ -22,15 +23,22 @@ ExternalProcessingFilterConfig::createFilterFactoryFromProtoTyped( proto_config, std::chrono::milliseconds(message_timeout_ms), max_message_timeout_ms, dual_info.scope, stats_prefix, dual_info.is_upstream, Envoy::Extensions::Filters::Common::Expr::getBuilder(context), context); - - return [filter_config = std::move(filter_config), grpc_service = proto_config.grpc_service(), - &context, dual_info](Http::FilterChainFactoryCallbacks& callbacks) { - auto client = std::make_unique( - context.clusterManager().grpcAsyncClientManager(), dual_info.scope); - - callbacks.addStreamFilter(Http::StreamFilterSharedPtr{ - std::make_shared(filter_config, std::move(client), grpc_service)}); - }; + if (proto_config.has_grpc_service()) { + return [filter_config = std::move(filter_config), &context, + dual_info](Http::FilterChainFactoryCallbacks& callbacks) { + auto client = std::make_unique( + context.clusterManager().grpcAsyncClientManager(), dual_info.scope); + callbacks.addStreamFilter( + Http::StreamFilterSharedPtr{std::make_shared(filter_config, std::move(client))}); + }; + } else { + return [proto_config = std::move(proto_config), filter_config = std::move(filter_config), + &context](Http::FilterChainFactoryCallbacks& callbacks) { + auto client = std::make_unique(proto_config, context); + callbacks.addStreamFilter( + Http::StreamFilterSharedPtr{std::make_shared(filter_config, std::move(client))}); + }; + } } Router::RouteSpecificFilterConfigConstSharedPtr @@ -54,14 +62,22 @@ ExternalProcessingFilterConfig::createFilterFactoryFromProtoWithServerContextTyp server_context.scope(), stats_prefix, false, Envoy::Extensions::Filters::Common::Expr::getBuilder(server_context), server_context); - return [filter_config = std::move(filter_config), grpc_service = proto_config.grpc_service(), - &server_context](Http::FilterChainFactoryCallbacks& callbacks) { - auto client = std::make_unique( - server_context.clusterManager().grpcAsyncClientManager(), server_context.scope()); - - callbacks.addStreamFilter(Http::StreamFilterSharedPtr{ - std::make_shared(filter_config, std::move(client), grpc_service)}); - }; + if (proto_config.has_grpc_service()) { + return [filter_config = std::move(filter_config), + &server_context](Http::FilterChainFactoryCallbacks& callbacks) { + auto client = std::make_unique( + server_context.clusterManager().grpcAsyncClientManager(), server_context.scope()); + callbacks.addStreamFilter( + Http::StreamFilterSharedPtr{std::make_shared(filter_config, std::move(client))}); + }; + } else { + return [proto_config = std::move(proto_config), filter_config = std::move(filter_config), + &server_context](Http::FilterChainFactoryCallbacks& callbacks) { + auto client = std::make_unique(proto_config, server_context); + callbacks.addStreamFilter( + Http::StreamFilterSharedPtr{std::make_shared(filter_config, std::move(client))}); + }; + } } LEGACY_REGISTER_FACTORY(ExternalProcessingFilterConfig, diff --git a/source/extensions/filters/http/ext_proc/ext_proc.cc b/source/extensions/filters/http/ext_proc/ext_proc.cc index 160f51910f..e087cdb159 100644 --- a/source/extensions/filters/http/ext_proc/ext_proc.cc +++ b/source/extensions/filters/http/ext_proc/ext_proc.cc @@ -9,6 +9,7 @@ #include "source/common/http/utility.h" #include "source/common/protobuf/utility.h" #include "source/common/runtime/runtime_features.h" +#include "source/extensions/filters/http/ext_proc/http_client/http_client_impl.h" #include "source/extensions/filters/http/ext_proc/mutation_utils.h" #include "absl/strings/str_format.h" @@ -21,6 +22,7 @@ namespace ExternalProcessing { namespace { using envoy::config::common::mutation_rules::v3::HeaderMutationRules; +using envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor; using envoy::extensions::filters::http::ext_proc::v3::ExtProcPerRoute; using envoy::extensions::filters::http::ext_proc::v3::ProcessingMode; using envoy::type::v3::StatusCode; @@ -49,6 +51,19 @@ absl::optional initProcessingMode(const ExtProcPerRoute& config) return absl::nullopt; } +absl::optional +getFilterGrpcService(const ExternalProcessor& config) { + if (config.has_grpc_service() != config.has_http_service()) { + if (config.has_grpc_service()) { + return config.grpc_service(); + } + } else { + throw EnvoyException("One and only one of grpc_service or http_service must be configured"); + } + + return absl::nullopt; +} + absl::optional initGrpcService(const ExtProcPerRoute& config) { if (config.has_overrides() && config.overrides().has_grpc_service()) { @@ -177,18 +192,19 @@ ProcessingMode allDisabledMode() { } // namespace -FilterConfig::FilterConfig( - const envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor& config, - const std::chrono::milliseconds message_timeout, const uint32_t max_message_timeout_ms, - Stats::Scope& scope, const std::string& stats_prefix, bool is_upstream, - Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr builder, - Server::Configuration::CommonFactoryContext& context) +FilterConfig::FilterConfig(const ExternalProcessor& config, + const std::chrono::milliseconds message_timeout, + const uint32_t max_message_timeout_ms, Stats::Scope& scope, + const std::string& stats_prefix, bool is_upstream, + Extensions::Filters::Common::Expr::BuilderInstanceSharedPtr builder, + Server::Configuration::CommonFactoryContext& context) : failure_mode_allow_(config.failure_mode_allow()), observability_mode_(config.observability_mode()), route_cache_action_(config.route_cache_action()), deferred_close_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, deferred_close_timeout, DEFAULT_DEFERRED_CLOSE_TIMEOUT_MS)), message_timeout_(message_timeout), max_message_timeout_ms_(max_message_timeout_ms), + grpc_service_(getFilterGrpcService(config)), send_body_without_waiting_for_header_response_( config.send_body_without_waiting_for_header_response()), stats_(generateStats(stats_prefix, config.stat_prefix(), scope)), @@ -215,14 +231,22 @@ FilterConfig::FilterConfig( config.response_attributes()), immediate_mutation_checker_(context.regexEngine()), thread_local_stream_manager_slot_(context.threadLocal().allocateSlot()) { - if (config.disable_clear_route_cache() && - (route_cache_action_ != - envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor::DEFAULT)) { + if (!grpc_service_.has_value()) { + // In case http_service configured, the processing mode can only support sending headers. + if (processing_mode_.request_body_mode() != ProcessingMode::NONE || + processing_mode_.response_body_mode() != ProcessingMode::NONE || + processing_mode_.request_trailer_mode() == ProcessingMode::SEND || + processing_mode_.response_trailer_mode() == ProcessingMode::SEND) { + throw EnvoyException( + "If http_service is configured, processing modes can not send any body or trailer."); + } + } + if (config.disable_clear_route_cache() && (route_cache_action_ != ExternalProcessor::DEFAULT)) { throw EnvoyException("disable_clear_route_cache and route_cache_action can not " "be set to none-default at the same time."); } if (config.disable_clear_route_cache()) { - route_cache_action_ = envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor::RETAIN; + route_cache_action_ = ExternalProcessor::RETAIN; } thread_local_stream_manager_slot_->set( [](Envoy::Event::Dispatcher&) { return std::make_shared(); }); @@ -333,6 +357,44 @@ void Filter::setEncoderFilterCallbacks(Http::StreamEncoderFilterCallbacks& callb watermark_callbacks_.setEncoderFilterCallbacks(&callbacks); } +void Filter::sendRequest(ProcessingRequest&& req, bool end_stream) { + // Calling the client send function to send the request. + client_->sendRequest(std::move(req), end_stream, filter_callbacks_->streamId(), this, stream_); +} + +void Filter::onComplete(ProcessingResponse& response) { + ENVOY_LOG(debug, "Received successful response from server"); + std::unique_ptr resp_ptr = std::make_unique(response); + onReceiveMessage(std::move(resp_ptr)); +} + +void Filter::onError() { + ENVOY_LOG(debug, "Received Error response from server"); + stats_.http_not_ok_resp_received_.inc(); + + if (processing_complete_) { + ENVOY_LOG(debug, "Ignoring stream message received after processing complete"); + return; + } + + if (config_->failureModeAllow()) { + // The user would like a none-200-ok response to not cause message processing to fail. + // Close the external processing. + processing_complete_ = true; + stats_.failure_mode_allowed_.inc(); + clearAsyncState(); + } else { + // Return an error and stop processing the current stream. + processing_complete_ = true; + decoding_state_.onFinishProcessorCall(Grpc::Status::Aborted); + encoding_state_.onFinishProcessorCall(Grpc::Status::Aborted); + ImmediateResponse errorResponse; + errorResponse.mutable_status()->set_code(StatusCode::InternalServerError); + errorResponse.set_details(absl::StrCat(ErrorPrefix, "_HTTP_ERROR")); + sendImmediateResponse(errorResponse); + } +} + Filter::StreamOpenState Filter::openStream() { // External processing is completed. This means there is no need to send any further // message to the server for processing. Just return IgnoreError so the filter @@ -341,7 +403,12 @@ Filter::StreamOpenState Filter::openStream() { ENVOY_LOG(debug, "External processing is completed when trying to open the gRPC stream"); return StreamOpenState::IgnoreError; } - if (!client_->stream()) { + + if (!config().grpcService().has_value()) { + return StreamOpenState::Ok; + } + + if (!stream_) { ENVOY_LOG(debug, "Opening gRPC stream to external processor"); Http::AsyncClient::ParentContext grpc_context; @@ -351,8 +418,9 @@ Filter::StreamOpenState Filter::openStream() { .setParentContext(grpc_context) .setBufferBodyForRetry(grpc_service_.has_retry_policy()); + ExternalProcessorClient* grpc_client = dynamic_cast(client_.get()); ExternalProcessorStreamPtr stream_object = - client_->start(*this, config_with_hash_key_, options, watermark_callbacks_); + grpc_client->start(*this, config_with_hash_key_, options, watermark_callbacks_); if (processing_complete_) { // Stream failed while starting and either onGrpcError or onGrpcClose was already called @@ -363,26 +431,29 @@ Filter::StreamOpenState Filter::openStream() { } stats_.streams_started_.inc(); - ExternalProcessorStream* stream = config_->threadLocalStreamManager().store( - std::move(stream_object), config_->stats(), config_->deferredCloseTimeout()); - client_->setStream(stream); + stream_ = config_->threadLocalStreamManager().store(std::move(stream_object), config_->stats(), + config_->deferredCloseTimeout()); // For custom access logging purposes. Applicable only for Envoy gRPC as Google gRPC does not // have a proper implementation of streamInfo. if (grpc_service_.has_envoy_grpc() && logging_info_ != nullptr) { - logging_info_->setClusterInfo(client_->stream()->streamInfo().upstreamClusterInfo()); + logging_info_->setClusterInfo(stream_->streamInfo().upstreamClusterInfo()); } } return StreamOpenState::Ok; } void Filter::closeStream() { - if (client_->stream()) { + if (!config_->grpcService().has_value()) { + return; + } + + if (stream_) { ENVOY_LOG(debug, "Calling close on stream"); - if (client_->stream()->close()) { + if (stream_->close()) { stats_.streams_closed_.inc(); } - config_->threadLocalStreamManager().erase(client_->stream()); - client_->setStream(nullptr); + config_->threadLocalStreamManager().erase(stream_); + stream_ = nullptr; } else { ENVOY_LOG(debug, "Stream already closed"); } @@ -390,8 +461,7 @@ void Filter::closeStream() { void Filter::deferredCloseStream() { ENVOY_LOG(debug, "Calling deferred close on stream"); - config_->threadLocalStreamManager().deferredErase(client_->stream(), - filter_callbacks_->dispatcher()); + config_->threadLocalStreamManager().deferredErase(stream_, filter_callbacks_->dispatcher()); } void Filter::onDestroy() { @@ -402,6 +472,11 @@ void Filter::onDestroy() { decoding_state_.stopMessageTimer(); encoding_state_.stopMessageTimer(); + if (!config_->grpcService().has_value()) { + client_->cancel(); + return; + } + if (config_->observabilityMode()) { // In observability mode where the main stream processing and side stream processing are // asynchronous, it is possible that filter instance is destroyed before the side stream request @@ -409,8 +484,8 @@ void Filter::onDestroy() { // closure is deferred upon filter destruction with a timer. // First, release the referenced filter resource. - if (client_->stream() != nullptr) { - client_->stream()->notifyFilterDestroy(); + if (stream_ != nullptr) { + stream_->notifyFilterDestroy(); } // Second, perform stream deferred closure. @@ -440,7 +515,7 @@ FilterHeadersStatus Filter::onHeaders(ProcessorState& state, state.onStartProcessorCall(std::bind(&Filter::onMessageTimeout, this), config_->messageTimeout(), ProcessorState::CallbackState::HeadersCallback); ENVOY_LOG(debug, "Sending headers message"); - client_->stream()->send(std::move(req), false); + sendRequest(std::move(req), false); stats_.stream_msgs_sent_.inc(); state.setPaused(true); return FilterHeadersStatus::StopIteration; @@ -673,7 +748,7 @@ Filter::sendHeadersInObservabilityMode(Http::RequestOrResponseHeaderMap& headers ProcessingRequest req = buildHeaderRequest(state, headers, end_stream, /*observability_mode=*/true); ENVOY_LOG(debug, "Sending headers message in observability mode"); - client_->stream()->send(std::move(req), false); + sendRequest(std::move(req), false); stats_.stream_msgs_sent_.inc(); return FilterHeadersStatus::Continue; @@ -698,7 +773,7 @@ Http::FilterDataStatus Filter::sendDataInObservabilityMode(Buffer::Instance& dat // Set up the the body chunk and send. auto req = setupBodyChunk(state, data, end_stream); req.set_observability_mode(true); - client_->stream()->send(std::move(req), false); + sendRequest(std::move(req), false); stats_.stream_msgs_sent_.inc(); ENVOY_LOG(debug, "Sending body message in ObservabilityMode"); } else if (state.bodyMode() != ProcessingMode::NONE) { @@ -890,7 +965,7 @@ void Filter::sendBodyChunk(ProcessorState& state, ProcessorState::CallbackState ProcessingRequest& req) { state.onStartProcessorCall(std::bind(&Filter::onMessageTimeout, this), config_->messageTimeout(), new_state); - client_->stream()->send(std::move(req), false); + sendRequest(std::move(req), false); stats_.stream_msgs_sent_.inc(); } @@ -906,21 +981,24 @@ void Filter::sendTrailers(ProcessorState& state, const Http::HeaderMap& trailers state.onStartProcessorCall(std::bind(&Filter::onMessageTimeout, this), config_->messageTimeout(), ProcessorState::CallbackState::TrailersCallback); ENVOY_LOG(debug, "Sending trailers message"); - client_->stream()->send(std::move(req), false); + sendRequest(std::move(req), false); stats_.stream_msgs_sent_.inc(); } void Filter::logGrpcStreamInfo() { - if (client_->stream() != nullptr && logging_info_ != nullptr && grpc_service_.has_envoy_grpc()) { - const auto& upstream_meter = client_->stream()->streamInfo().getUpstreamBytesMeter(); + if (!config().grpcService().has_value()) { + return; + } + + if (stream_ != nullptr && logging_info_ != nullptr && grpc_service_.has_envoy_grpc()) { + const auto& upstream_meter = stream_->streamInfo().getUpstreamBytesMeter(); if (upstream_meter != nullptr) { logging_info_->setBytesSent(upstream_meter->wireBytesSent()); logging_info_->setBytesReceived(upstream_meter->wireBytesReceived()); } // Only set upstream host in logging info once. if (logging_info_->upstreamHost() == nullptr) { - logging_info_->setUpstreamHost( - client_->stream()->streamInfo().upstreamInfo()->upstreamHost()); + logging_info_->setUpstreamHost(stream_->streamInfo().upstreamInfo()->upstreamHost()); } } } diff --git a/source/extensions/filters/http/ext_proc/ext_proc.h b/source/extensions/filters/http/ext_proc/ext_proc.h index ce5a9ed4b1..240ffc505e 100644 --- a/source/extensions/filters/http/ext_proc/ext_proc.h +++ b/source/extensions/filters/http/ext_proc/ext_proc.h @@ -24,6 +24,7 @@ #include "source/extensions/filters/common/mutation_rules/mutation_rules.h" #include "source/extensions/filters/http/common/pass_through_filter.h" #include "source/extensions/filters/http/ext_proc/client.h" +#include "source/extensions/filters/http/ext_proc/client_base.h" #include "source/extensions/filters/http/ext_proc/matching_utils.h" #include "source/extensions/filters/http/ext_proc/processor_state.h" @@ -47,7 +48,8 @@ namespace ExternalProcessing { COUNTER(clear_route_cache_ignored) \ COUNTER(clear_route_cache_disabled) \ COUNTER(clear_route_cache_upstream_ignored) \ - COUNTER(send_immediate_resp_upstream_ignored) + COUNTER(send_immediate_resp_upstream_ignored) \ + COUNTER(http_not_ok_resp_received) struct ExtProcFilterStats { ALL_EXT_PROC_FILTER_STATS(GENERATE_COUNTER_STRUCT) @@ -274,6 +276,10 @@ class FilterConfig { return thread_local_stream_manager_slot_->getTyped(); } + const absl::optional grpcService() const { + return grpc_service_; + } + private: ExtProcFilterStats generateStats(const std::string& prefix, const std::string& filter_stats_prefix, Stats::Scope& scope) { @@ -287,6 +293,7 @@ class FilterConfig { const std::chrono::milliseconds deferred_close_timeout_; const std::chrono::milliseconds message_timeout_; const uint32_t max_message_timeout_ms_; + const absl::optional grpc_service_; const bool send_body_without_waiting_for_header_response_; ExtProcFilterStats stats_; @@ -379,10 +386,11 @@ class Filter : public Logger::Loggable, }; public: - Filter(const FilterConfigSharedPtr& config, ExternalProcessorClientPtr&& client, - const envoy::config::core::v3::GrpcService& grpc_service) + Filter(const FilterConfigSharedPtr& config, ClientBasePtr&& client) : config_(config), client_(std::move(client)), stats_(config->stats()), - grpc_service_(grpc_service), config_with_hash_key_(grpc_service), + grpc_service_(config->grpcService().has_value() ? config->grpcService().value() + : envoy::config::core::v3::GrpcService()), + config_with_hash_key_(grpc_service_), decoding_state_(*this, config->processingMode(), config->untypedForwardingMetadataNamespaces(), config->typedForwardingMetadataNamespaces(), @@ -444,6 +452,8 @@ class Filter : public Logger::Loggable, const ProcessorState& encodingState() { return encoding_state_; } const ProcessorState& decodingState() { return decoding_state_; } + void onComplete(envoy::service::ext_proc::v3::ProcessingResponse& response) override; + void onError() override; private: void mergePerRouteConfig(); @@ -480,8 +490,10 @@ class Filter : public Logger::Loggable, buildHeaderRequest(ProcessorState& state, Http::RequestOrResponseHeaderMap& headers, bool end_stream, bool observability_mode); + void sendRequest(envoy::service::ext_proc::v3::ProcessingRequest&& req, bool end_stream); + const FilterConfigSharedPtr config_; - const ExternalProcessorClientPtr client_; + const ClientBasePtr client_; ExtProcFilterStats stats_; ExtProcLoggingInfo* logging_info_; envoy::config::core::v3::GrpcService grpc_service_; @@ -491,6 +503,10 @@ class Filter : public Logger::Loggable, DecodingProcessorState decoding_state_; EncodingProcessorState encoding_state_; + // The gRPC stream to the external processor, which will be opened + // when it's time to send the first message. + ExternalProcessorStream* stream_ = nullptr; + // Set to true when no more messages need to be sent to the processor. // This happens when the processor has closed the stream, or when it has // failed. diff --git a/source/extensions/filters/http/ext_proc/http_client/BUILD b/source/extensions/filters/http/ext_proc/http_client/BUILD index 16afa5f698..681afb3bfd 100644 --- a/source/extensions/filters/http/ext_proc/http_client/BUILD +++ b/source/extensions/filters/http/ext_proc/http_client/BUILD @@ -8,22 +8,17 @@ licenses(["notice"]) # Apache 2 envoy_extension_package() -envoy_cc_library( - name = "client_base_interface", - hdrs = ["client_base.h"], - tags = ["skip_on_windows"], - deps = [], -) - envoy_cc_library( name = "http_client_lib", srcs = ["http_client_impl.cc"], hdrs = ["http_client_impl.h"], tags = ["skip_on_windows"], deps = [ - "client_base_interface", "//source/common/common:enum_to_int", + "//source/common/http:header_map_lib", "//source/common/http:utility_lib", - "//source/extensions/filters/http/ext_proc", + "//source/extensions/filters/http/ext_proc:client_base_interface", + "@envoy_api//envoy/extensions/filters/http/ext_proc/v3:pkg_cc_proto", + "@envoy_api//envoy/service/ext_proc/v3:pkg_cc_proto", ], ) diff --git a/source/extensions/filters/http/ext_proc/http_client/client_base.h b/source/extensions/filters/http/ext_proc/http_client/client_base.h deleted file mode 100644 index fd9ae5ce7c..0000000000 --- a/source/extensions/filters/http/ext_proc/http_client/client_base.h +++ /dev/null @@ -1,33 +0,0 @@ -#pragma once - -#include - -namespace Envoy { -namespace Extensions { -namespace HttpFilters { -namespace ExternalProcessing { - -/** - * Async callbacks used during external processing. - */ -class RequestCallbacks { -public: - virtual ~RequestCallbacks() = default; - virtual void onComplete() PURE; -}; - -/** - * Async client base class used during external processing. - */ -class ClientBase { -public: - virtual ~ClientBase() = default; - - virtual void sendRequest() PURE; - virtual void cancel() PURE; -}; - -} // namespace ExternalProcessing -} // namespace HttpFilters -} // namespace Extensions -} // namespace Envoy diff --git a/source/extensions/filters/http/ext_proc/http_client/http_client_impl.cc b/source/extensions/filters/http/ext_proc/http_client/http_client_impl.cc index abc1721923..b333ea513c 100644 --- a/source/extensions/filters/http/ext_proc/http_client/http_client_impl.cc +++ b/source/extensions/filters/http/ext_proc/http_client/http_client_impl.cc @@ -1,6 +1,8 @@ #include "source/extensions/filters/http/ext_proc/http_client/http_client_impl.h" #include "source/common/common/enum_to_int.h" +#include "source/common/http/header_map_impl.h" +#include "source/common/http/message_impl.h" #include "source/common/http/utility.h" namespace Envoy { @@ -8,13 +10,90 @@ namespace Extensions { namespace HttpFilters { namespace ExternalProcessing { +namespace { +Http::RequestMessagePtr buildHttpRequest(absl::string_view uri, const uint64_t stream_id, + absl::string_view req_in_json) { + absl::string_view host, path; + Envoy::Http::Utility::extractHostPathFromUri(uri, host, path); + ENVOY_LOG_MISC(debug, " Ext_Proc HTTP client send request to uri {}, host {}, path {}", uri, host, + path); + + // Construct a HTTP POST message. + const Envoy::Http::HeaderValues& header_values = Envoy::Http::Headers::get(); + Http::RequestHeaderMapPtr headers = + Envoy::Http::createHeaderMap( + {{header_values.Method, "POST"}, + {header_values.Scheme, "http"}, + {header_values.Path, std::string(path)}, + {header_values.ContentType, "application/json"}, + {header_values.RequestId, std::to_string(stream_id)}, + {header_values.Host, std::string(host)}}); + Http::RequestMessagePtr message = + std::make_unique(std::move(headers)); + message->body().add(req_in_json); + return message; +} + +} // namespace + +void ExtProcHttpClient::sendRequest(envoy::service::ext_proc::v3::ProcessingRequest&& req, bool, + const uint64_t stream_id, RequestCallbacks* callbacks, + StreamBase*) { + // Cancel any active requests. + cancel(); + callbacks_ = callbacks; + + // Transcode req message into JSON string. + auto req_in_json = MessageUtil::getJsonStringFromMessage(req); + if (req_in_json.ok()) { + const auto http_uri = config_.http_service().http_service().http_uri(); + Http::RequestMessagePtr message = + buildHttpRequest(http_uri.uri(), stream_id, req_in_json.value()); + auto options = Http::AsyncClient::RequestOptions() + .setTimeout(std::chrono::milliseconds( + DurationUtil::durationToMilliseconds(http_uri.timeout()))) + .setSendInternal(false) + .setSendXff(false); + const std::string cluster = http_uri.cluster(); + const auto thread_local_cluster = context().clusterManager().getThreadLocalCluster(cluster); + if (thread_local_cluster) { + active_request_ = + thread_local_cluster->httpAsyncClient().send(std::move(message), *this, options); + } else { + ENVOY_LOG(error, "ext_proc cluster {} does not exist in the config", cluster); + } + } +} + void ExtProcHttpClient::onSuccess(const Http::AsyncClient::Request&, Http::ResponseMessagePtr&& response) { auto status = Envoy::Http::Utility::getResponseStatusOrNullopt(response->headers()); + active_request_ = nullptr; if (status.has_value()) { uint64_t status_code = status.value(); if (status_code == Envoy::enumToInt(Envoy::Http::Code::OK)) { - ENVOY_LOG(error, "Response status is OK"); + std::string msg_body = response->body().toString(); + ENVOY_LOG(debug, "Response status is OK, message body length {}", msg_body.size()); + envoy::service::ext_proc::v3::ProcessingResponse response_msg; + if (!msg_body.empty()) { + bool has_unknown_field; + auto status = MessageUtil::loadFromJsonNoThrow(msg_body, response_msg, has_unknown_field); + if (!status.ok()) { + ENVOY_LOG( + error, + "The HTTP response body can not be decoded into a ProcessResponse proto message"); + onError(); + return; + } + } else { + ENVOY_LOG(error, "Response body is empty"); + onError(); + return; + } + if (callbacks_) { + callbacks_->onComplete(response_msg); + callbacks_ = nullptr; + } } else { ENVOY_LOG(error, "Response status is not OK, status: {}", status_code); onError(); @@ -31,12 +110,25 @@ void ExtProcHttpClient::onFailure(const Http::AsyncClient::Request&, ASSERT(reason == Http::AsyncClient::FailureReason::Reset || reason == Http::AsyncClient::FailureReason::ExceedResponseBufferLimit); ENVOY_LOG(error, "Request failed: stream has been reset"); + active_request_ = nullptr; onError(); } void ExtProcHttpClient::onError() { // Cancel if the request is active. cancel(); + ENVOY_LOG(error, "ext_proc HTTP client error condition happens."); + if (callbacks_) { + callbacks_->onError(); + callbacks_ = nullptr; + } +} + +void ExtProcHttpClient::cancel() { + if (active_request_) { + active_request_->cancel(); + active_request_ = nullptr; + } } } // namespace ExternalProcessing diff --git a/source/extensions/filters/http/ext_proc/http_client/http_client_impl.h b/source/extensions/filters/http/ext_proc/http_client/http_client_impl.h index fa2df5afd1..6d9aace5ad 100644 --- a/source/extensions/filters/http/ext_proc/http_client/http_client_impl.h +++ b/source/extensions/filters/http/ext_proc/http_client/http_client_impl.h @@ -2,11 +2,12 @@ #include +#include "envoy/extensions/filters/http/ext_proc/v3/ext_proc.pb.h" #include "envoy/http/async_client.h" +#include "envoy/service/ext_proc/v3/external_processor.pb.h" #include "source/common/common/logger.h" -#include "source/extensions/filters/http/ext_proc/ext_proc.h" -#include "source/extensions/filters/http/ext_proc/http_client/client_base.h" +#include "source/extensions/filters/http/ext_proc/client_base.h" namespace Envoy { namespace Extensions { @@ -23,8 +24,10 @@ class ExtProcHttpClient : public ClientBase, ~ExtProcHttpClient() { cancel(); } - void sendRequest() override {} - void cancel() override {} + void sendRequest(envoy::service::ext_proc::v3::ProcessingRequest&& req, bool end_stream, + const uint64_t stream_id, RequestCallbacks* callbacks, + StreamBase* stream) override; + void cancel() override; void onBeforeFinalizeUpstreamSpan(Tracing::Span&, const Http::ResponseHeaderMap*) override {} // Http::AsyncClient::Callbacks implemented by this class. @@ -39,6 +42,8 @@ class ExtProcHttpClient : public ClientBase, void onError(); envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor config_; Server::Configuration::ServerFactoryContext& context_; + Http::AsyncClient::Request* active_request_{}; + RequestCallbacks* callbacks_{}; }; } // namespace ExternalProcessing diff --git a/source/server/admin/stats_html_render.cc b/source/server/admin/stats_html_render.cc index f86c84861f..954b685222 100644 --- a/source/server/admin/stats_html_render.cc +++ b/source/server/admin/stats_html_render.cc @@ -78,7 +78,7 @@ void StatsHtmlRender::noStats(Buffer::Instance& response, absl::string_view type void StatsHtmlRender::generate(Buffer::Instance& response, const std::string& name, const Stats::ParentHistogram& histogram) { if (json_histograms_) { - Json::Streamer streamer(response); + Json::BufferStreamer streamer(response); // If this is the first histogram we are rendering, then we need to first // generate the supported-percentiles array sand save it in a constant. diff --git a/source/server/admin/stats_render.cc b/source/server/admin/stats_render.cc index ad46c3f39b..4e7652c8a5 100644 --- a/source/server/admin/stats_render.cc +++ b/source/server/admin/stats_render.cc @@ -156,7 +156,7 @@ void StatsJsonRender::generate(Buffer::Instance& response, const std::string& na switch (histogram_buckets_mode_) { case Utility::HistogramBucketsMode::Unset: case Utility::HistogramBucketsMode::Summary: { - Json::Streamer::MapPtr map = json_->histogram_array_->addMap(); + Json::BufferStreamer::MapPtr map = json_->histogram_array_->addMap(); map->addEntries({{"name", name}}); map->addKey("values"); populatePercentiles(histogram, *map); @@ -186,10 +186,10 @@ void StatsJsonRender::generate(Buffer::Instance& response, const std::string& na drainIfNeeded(response); } -void StatsJsonRender::populateSupportedPercentiles(Json::Streamer::Array& array) { +void StatsJsonRender::populateSupportedPercentiles(Json::BufferStreamer::Array& array) { Stats::HistogramStatisticsImpl empty_statistics; std::vector supported = empty_statistics.supportedQuantiles(); - std::vector views(supported.size()); + std::vector views(supported.size()); for (uint32_t i = 0, n = supported.size(); i < n; ++i) { views[i] = supported[i] * 100; } @@ -197,8 +197,8 @@ void StatsJsonRender::populateSupportedPercentiles(Json::Streamer::Array& array) } void StatsJsonRender::populatePercentiles(const Stats::ParentHistogram& histogram, - Json::Streamer::Map& map) { - Json::Streamer::ArrayPtr array = map.addArray(); + Json::BufferStreamer::Map& map) { + Json::BufferStreamer::ArrayPtr array = map.addArray(); std::vector totals = histogram.cumulativeStatistics().computedQuantiles(), intervals = histogram.intervalStatistics().computedQuantiles(); uint32_t min_size = std::min(totals.size(), intervals.size()); @@ -238,7 +238,7 @@ void StatsJsonRender::renderHistogramStart() { void StatsJsonRender::generateHistogramDetail(const std::string& name, const Stats::ParentHistogram& histogram, - Json::Streamer::Map& map) { + Json::BufferStreamer::Map& map) { // Now we produce the stream-able histogram records, without using the json intermediate // representation or serializer. map.addEntries({{"name", name}}); @@ -251,8 +251,8 @@ void StatsJsonRender::generateHistogramDetail(const std::string& name, } void StatsJsonRender::populateBucketsVerbose( - const std::vector& buckets, Json::Streamer::Map& map) { - Json::Streamer::ArrayPtr buckets_array = map.addArray(); + const std::vector& buckets, Json::BufferStreamer::Map& map) { + Json::BufferStreamer::ArrayPtr buckets_array = map.addArray(); for (const Stats::ParentHistogram::Bucket& bucket : buckets) { buckets_array->addMap()->addEntries( {{"lower_bound", bucket.lower_bound_}, {"width", bucket.width_}, {"count", bucket.count_}}); @@ -281,12 +281,12 @@ void StatsJsonRender::collectBuckets(const std::string& name, size_t min_size = std::min({interval_buckets.size(), cumulative_buckets.size(), supported_buckets.size()}); - Json::Streamer::MapPtr map = json_->histogram_array_->addMap(); + Json::BufferStreamer::MapPtr map = json_->histogram_array_->addMap(); map->addEntries({{"name", name}}); map->addKey("buckets"); - Json::Streamer::ArrayPtr buckets = map->addArray(); + Json::BufferStreamer::ArrayPtr buckets = map->addArray(); for (uint32_t i = 0; i < min_size; ++i) { - Json::Streamer::MapPtr bucket_map = buckets->addMap(); + Json::BufferStreamer::MapPtr bucket_map = buckets->addMap(); bucket_map->addEntries({{"upper_bound", supported_buckets[i]}, {"interval", interval_buckets[i]}, {"cumulative", cumulative_buckets[i]}}); diff --git a/source/server/admin/stats_render.h b/source/server/admin/stats_render.h index 8894c9ee3d..f78ba42f9a 100644 --- a/source/server/admin/stats_render.h +++ b/source/server/admin/stats_render.h @@ -83,7 +83,7 @@ class StatsJsonRender : public StatsRender { * * @param array the json streaming array array to stream into. */ - static void populateSupportedPercentiles(Json::Streamer::Array& array); + static void populateSupportedPercentiles(Json::BufferStreamer::Array& array); /** * Streams detail about the provided histogram into the provided JSON map. @@ -98,7 +98,7 @@ class StatsJsonRender : public StatsRender { */ static void generateHistogramDetail(const std::string& name, const Stats::ParentHistogram& histogram, - Json::Streamer::Map& map); + Json::BufferStreamer::Map& map); private: // Collects the buckets from the specified histogram. @@ -107,10 +107,10 @@ class StatsJsonRender : public StatsRender { const std::vector& cumulative_buckets); static void populateBucketsVerbose(const std::vector& buckets, - Json::Streamer::Map& map); + Json::BufferStreamer::Map& map); void renderHistogramStart(); static void populatePercentiles(const Stats::ParentHistogram& histogram, - Json::Streamer::Map& map); + Json::BufferStreamer::Map& map); // This function irons out an API mistake made when defining the StatsRender // interface. The issue is that callers can provide a response buffer when @@ -138,12 +138,12 @@ class StatsJsonRender : public StatsRender { // before the output stream is considered complete. struct JsonContext { explicit JsonContext(Buffer::Instance& response); - Json::Streamer streamer_; - Json::Streamer::MapPtr stats_map_; - Json::Streamer::ArrayPtr stats_array_; - Json::Streamer::MapPtr histogram_map1_; - Json::Streamer::MapPtr histogram_map2_; - Json::Streamer::ArrayPtr histogram_array_; + Json::BufferStreamer streamer_; + Json::BufferStreamer::MapPtr stats_map_; + Json::BufferStreamer::ArrayPtr stats_array_; + Json::BufferStreamer::MapPtr histogram_map1_; + Json::BufferStreamer::MapPtr histogram_map2_; + Json::BufferStreamer::ArrayPtr histogram_array_; }; std::unique_ptr json_; Buffer::Instance& response_; diff --git a/source/server/server.cc b/source/server/server.cc index 826c6dc491..d29713c65a 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -720,6 +720,8 @@ absl::Status InstanceBase::initializeOrThrow(Network::Address::InstanceConstShar auto typed_admin = dynamic_cast(admin_.get()); RELEASE_ASSERT(typed_admin != nullptr, "Admin implementation is not an AdminImpl."); initial_config.initAdminAccessLog(bootstrap_, typed_admin->factoryContext()); + ENVOY_LOG(info, "Starting admin HTTP server at {}", + initial_config.admin().address()->asString()); admin_->startHttpListener(initial_config.admin().accessLogs(), initial_config.admin().address(), initial_config.admin().socketOptions()); #else diff --git a/test/common/config/BUILD b/test/common/config/BUILD index bc68571f94..9111a90196 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -144,6 +144,7 @@ envoy_cc_test( "//test/mocks/upstream:thread_local_cluster_mocks", "//test/test_common:environment_lib", "//test/test_common:logging_lib", + "//test/test_common:status_utility_lib", "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", "@com_github_cncf_xds//udpa/type/v1:pkg_cc_proto", diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc index b3945df1e6..98735c89c2 100644 --- a/test/common/config/utility_test.cc +++ b/test/common/config/utility_test.cc @@ -20,6 +20,7 @@ #include "test/mocks/upstream/thread_local_cluster.h" #include "test/test_common/environment.h" #include "test/test_common/logging.h" +#include "test/test_common/status_utility.h" #include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" @@ -29,6 +30,7 @@ #include "udpa/type/v1/typed_struct.pb.h" #include "xds/type/v3/typed_struct.pb.h" +using Envoy::StatusHelpers::HasStatusMessage; using testing::ContainsRegex; using testing::Eq; using testing::HasSubstr; @@ -894,6 +896,43 @@ TEST(UtilityTest, GetGrpcControlPlane) { } } +TEST(UtilityTest, ValidateTerminalFiltersSucceeds) { + EXPECT_OK(Utility::validateTerminalFilters("filter_name", "filter_type", "chain_type", + /*is_terminal_filter=*/true, + /*last_filter_in_current_config=*/true)); + EXPECT_OK(Utility::validateTerminalFilters("filter_name", "filter_type", "chain_type", + /*is_terminal_filter=*/false, + /*last_filter_in_current_config=*/false)); +} + +TEST(UtilityTest, ValidateTerminalFilterFailsWithMisplacedTerminalFilter) { + EXPECT_THAT( + Utility::validateTerminalFilters("filter_name", "filter_type", "chain_type", + /*is_terminal_filter=*/true, + /*last_filter_in_current_config=*/false), + HasStatusMessage("Error: terminal filter named filter_name of type filter_type must be the " + "last filter in a chain_type filter chain.")); +} + +TEST(UtilityTest, ValidateTerminalFilterFailsWithMissingTerminalFilter) { + EXPECT_THAT(Utility::validateTerminalFilters("filter_name", "filter_type", "chain_type", + /*is_terminal_filter=*/false, + /*last_filter_in_current_config=*/true), + HasStatusMessage("Error: non-terminal filter named filter_name of type " + "filter_type is the last filter in a chain_type filter chain.")); +} + +TEST(UtilityTest, ValidateTerminalFilterFailsWithMissingUpstreamTerminalFilter) { + EXPECT_THAT( + Utility::validateTerminalFilters("filter_name", "filter_type", "router upstream http", + /*is_terminal_filter=*/false, + /*last_filter_in_current_config=*/true), + HasStatusMessage("Error: non-terminal filter named filter_name of type " + "filter_type is the last filter in a router upstream http filter chain. " + "When upstream_http_filters are specified, they must explicitly end with an " + "UpstreamCodec filter.")); +} + } // namespace } // namespace Config } // namespace Envoy diff --git a/test/common/quic/client_connection_factory_impl_test.cc b/test/common/quic/client_connection_factory_impl_test.cc index 455a819bcb..5f505584b9 100644 --- a/test/common/quic/client_connection_factory_impl_test.cc +++ b/test/common/quic/client_connection_factory_impl_test.cc @@ -98,7 +98,7 @@ TEST_P(QuicNetworkConnectionTest, BufferLimits) { initialize(); std::unique_ptr client_connection = createQuicNetworkConnection( *quic_info_, crypto_config_, - quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false}, + quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT}, dispatcher_, test_address_, test_address_, quic_stat_names_, {}, *store_.rootScope(), nullptr, nullptr, connection_id_generator_, *factory_); EnvoyQuicClientSession* session = static_cast(client_connection.get()); @@ -125,7 +125,7 @@ TEST_P(QuicNetworkConnectionTest, SocketOptions) { std::unique_ptr client_connection = createQuicNetworkConnection( *quic_info_, crypto_config_, - quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false}, + quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT}, dispatcher_, test_address_, test_address_, quic_stat_names_, {}, *store_.rootScope(), socket_options, nullptr, connection_id_generator_, *factory_); EnvoyQuicClientSession* session = static_cast(client_connection.get()); @@ -142,7 +142,7 @@ TEST_P(QuicNetworkConnectionTest, LocalAddress) { : Network::Utility::getCanonicalIpv4LoopbackAddress(); std::unique_ptr client_connection = createQuicNetworkConnection( *quic_info_, crypto_config_, - quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false}, + quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT}, dispatcher_, test_address_, local_addr, quic_stat_names_, {}, *store_.rootScope(), nullptr, nullptr, connection_id_generator_, *factory_); EnvoyQuicClientSession* session = static_cast(client_connection.get()); @@ -164,7 +164,7 @@ TEST_P(QuicNetworkConnectionTest, Srtt) { std::unique_ptr client_connection = createQuicNetworkConnection( info, crypto_config_, - quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT, false}, + quic::QuicServerId{factory_->clientContextConfig()->serverNameIndication(), PEER_PORT}, dispatcher_, test_address_, test_address_, quic_stat_names_, rtt_cache, *store_.rootScope(), nullptr, nullptr, connection_id_generator_, *factory_); diff --git a/test/common/quic/envoy_quic_client_session_test.cc b/test/common/quic/envoy_quic_client_session_test.cc index d757f03a0b..8e100e5d77 100644 --- a/test/common/quic/envoy_quic_client_session_test.cc +++ b/test/common/quic/envoy_quic_client_session_test.cc @@ -137,7 +137,7 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam( quic_config_, quic_version_, std::unique_ptr(quic_connection_), - quic::QuicServerId("example.com", 443, false), crypto_config_, *dispatcher_, + quic::QuicServerId("example.com", 443), crypto_config_, *dispatcher_, /*send_buffer_limit*/ 1024 * 1024, crypto_stream_factory_, quic_stat_names_, cache, *store_.rootScope(), transport_socket_options_, uts_factory); @@ -676,8 +676,8 @@ TEST_P(EnvoyQuicClientSessionTest, WriteBlockedAndUnblock) { EnvoyQuicClientConnectionPeer::onFileEvent(*quic_connection_, Event::FileReadyType::Write, *quic_connection_->connectionSocket()); EXPECT_FALSE(quic_connection_->writer()->IsWriteBlocked()); - EXPECT_CALL(stream_callbacks, - onResetStream(Http::StreamResetReason::LocalReset, "QUIC_STREAM_REQUEST_REJECTED")); + EXPECT_CALL(stream_callbacks, onResetStream(Http::StreamResetReason::LocalReset, + "QUIC_STREAM_REQUEST_REJECTED|FROM_SELF")); EXPECT_CALL(*quic_connection_, SendControlFrame(_)); stream.resetStream(Http::StreamResetReason::LocalReset); } diff --git a/test/common/quic/envoy_quic_client_stream_test.cc b/test/common/quic/envoy_quic_client_stream_test.cc index e8fd682cad..6ae790c29d 100644 --- a/test/common/quic/envoy_quic_client_stream_test.cc +++ b/test/common/quic/envoy_quic_client_stream_test.cc @@ -210,6 +210,8 @@ TEST_F(EnvoyQuicClientStreamTest, GetRequestAndHeaderOnlyResponse) { const auto result = quic_stream_->encodeHeaders(request_headers_, /*end_stream=*/true); EXPECT_TRUE(result.ok()); + quic_stream_->setFlushTimeout(std::chrono::milliseconds(100)); // No-op + EXPECT_CALL(stream_decoder_, decodeHeaders_(_, /*end_stream=*/false)) .WillOnce(Invoke([](const Http::ResponseHeaderMapPtr& headers, bool) { EXPECT_EQ("200", headers->getStatusValue()); diff --git a/test/common/quic/test_utils.h b/test/common/quic/test_utils.h index c9c1d04a70..058f9d6d5c 100644 --- a/test/common/quic/test_utils.h +++ b/test/common/quic/test_utils.h @@ -205,7 +205,7 @@ class MockEnvoyQuicClientSession : public IsolatedStoreProvider, public EnvoyQui Event::Dispatcher& dispatcher, uint32_t send_buffer_limit, EnvoyQuicCryptoClientStreamFactoryInterface& crypto_stream_factory) : EnvoyQuicClientSession(config, supported_versions, std::move(connection), - quic::QuicServerId("example.com", 443, false), + quic::QuicServerId("example.com", 443), std::make_shared( quic::test::crypto_test_utils::ProofVerifierForTesting()), dispatcher, send_buffer_limit, crypto_stream_factory, diff --git a/test/common/router/scoped_config_impl_test.cc b/test/common/router/scoped_config_impl_test.cc index 37d8a7fa65..004b7b2562 100644 --- a/test/common/router/scoped_config_impl_test.cc +++ b/test/common/router/scoped_config_impl_test.cc @@ -384,19 +384,23 @@ TEST_F(ScopedRouteInfoTest, Creation) { // Tests that config hash changes if ScopedRouteConfiguration of the ScopedRouteInfo changes. TEST_F(ScopedRouteInfoTest, Hash) { - const envoy::config::route::v3::ScopedRouteConfiguration config_copy = scoped_route_config_; - info_ = std::make_unique(scoped_route_config_, route_config_); + envoy::config::route::v3::ScopedRouteConfiguration scoped_route_config1 = scoped_route_config_; + info_ = std::make_unique(std::move(scoped_route_config1), route_config_); EXPECT_EQ(info_->routeConfig().get(), route_config_.get()); - EXPECT_TRUE(TestUtility::protoEqual(info_->configProto(), config_copy)); + EXPECT_TRUE(TestUtility::protoEqual(info_->configProto(), scoped_route_config_)); EXPECT_EQ(info_->scopeName(), "foo_scope"); EXPECT_EQ(info_->scopeKey(), makeKey({"foo", "bar"})); - const auto info2 = std::make_unique(scoped_route_config_, route_config_); + envoy::config::route::v3::ScopedRouteConfiguration scoped_route_config2 = scoped_route_config_; + const auto info2 = + std::make_unique(std::move(scoped_route_config2), route_config_); ASSERT_EQ(info2->configHash(), info_->configHash()); // Mutate the config and hash should be different now. - scoped_route_config_.set_on_demand(true); - const auto info3 = std::make_unique(scoped_route_config_, route_config_); + envoy::config::route::v3::ScopedRouteConfiguration scoped_route_config3 = scoped_route_config_; + scoped_route_config3.set_on_demand(true); + const auto info3 = + std::make_unique(std::move(scoped_route_config3), route_config_); ASSERT_NE(info3->configHash(), info_->configHash()); } diff --git a/test/extensions/config_subscription/grpc/BUILD b/test/extensions/config_subscription/grpc/BUILD index b0f9ce75ed..466a14269b 100644 --- a/test/extensions/config_subscription/grpc/BUILD +++ b/test/extensions/config_subscription/grpc/BUILD @@ -32,6 +32,7 @@ envoy_cc_test( "//test/test_common:logging_lib", "//test/test_common:resources_lib", "//test/test_common:simulated_time_system_lib", + "//test/test_common:status_utility_lib", "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", @@ -64,6 +65,7 @@ envoy_cc_test( "//test/test_common:logging_lib", "//test/test_common:resources_lib", "//test/test_common:simulated_time_system_lib", + "//test/test_common:status_utility_lib", "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", @@ -180,6 +182,7 @@ envoy_cc_test( "//test/test_common:logging_lib", "//test/test_common:resources_lib", "//test/test_common:simulated_time_system_lib", + "//test/test_common:status_utility_lib", "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/api/v2:pkg_cc_proto", diff --git a/test/extensions/config_subscription/grpc/grpc_mux_failover_test.cc b/test/extensions/config_subscription/grpc/grpc_mux_failover_test.cc index 8a990f134f..2b611e1c7b 100644 --- a/test/extensions/config_subscription/grpc/grpc_mux_failover_test.cc +++ b/test/extensions/config_subscription/grpc/grpc_mux_failover_test.cc @@ -15,7 +15,6 @@ using testing::Return; namespace Envoy { namespace Config { -namespace { // Validates that if no failover is set, then all actions are essentially a pass // through. @@ -237,6 +236,13 @@ class GrpcMuxFailoverTest : public testing::Test { failover_callbacks_->onDiscoveryResponse(std::move(response), cp_stats); } + void invokeCloseStream() { + // A wrapper that invokes closeStream(). It is needed because closeStream() + // is a private method, and while this class is a friend for GrpcMuxFailover, + // the tests cannot invoke the method directly. + grpc_mux_failover_->closeStream(); + } + // Override a timer to emulate its expiration without waiting for it to expire. NiceMock dispatcher_; Event::MockTimer* timer_; @@ -626,26 +632,38 @@ TEST_F(GrpcMuxFailoverTest, OnWriteableConnectedToPrimaryInvoked) { // Validates that when connected to primary, a subsequent call to establishNewStream // will not try to recreate the stream. TEST_F(GrpcMuxFailoverTest, NoRecreateStreamWhenConnectedToPrimary) { - // Validate connected to primary. - { - connectToPrimary(); - EXPECT_CALL(primary_stream_, establishNewStream()).Times(0); - EXPECT_CALL(failover_stream_, establishNewStream()).Times(0); - grpc_mux_failover_->establishNewStream(); - } + connectToPrimary(); + EXPECT_CALL(primary_stream_, establishNewStream()).Times(0); + EXPECT_CALL(failover_stream_, establishNewStream()).Times(0); + grpc_mux_failover_->establishNewStream(); } // Validates that when connected to failover, a subsequent call to establishNewStream // will not try to recreate the stream. TEST_F(GrpcMuxFailoverTest, NoRecreateStreamWhenConnectedToFailover) { - // Validate connected to failover. - { - connectToFailover(); - EXPECT_CALL(primary_stream_, establishNewStream()).Times(0); - EXPECT_CALL(failover_stream_, establishNewStream()).Times(0); - grpc_mux_failover_->establishNewStream(); - } + connectToFailover(); + EXPECT_CALL(primary_stream_, establishNewStream()).Times(0); + EXPECT_CALL(failover_stream_, establishNewStream()).Times(0); + grpc_mux_failover_->establishNewStream(); +} + +// Validates that closing the stream when connected to primary closes the +// primary stream. +TEST_F(GrpcMuxFailoverTest, CloseStreamWhenConnectedToPrimary) { + connectToPrimary(); + EXPECT_CALL(primary_stream_, closeStream()); + EXPECT_CALL(failover_stream_, closeStream()).Times(0); + invokeCloseStream(); } -} // namespace + +// Validates that closing the stream when connected to failover closes the +// failover stream. +TEST_F(GrpcMuxFailoverTest, CloseStreamWhenConnectedToFailover) { + connectToFailover(); + EXPECT_CALL(primary_stream_, closeStream()).Times(0); + EXPECT_CALL(failover_stream_, closeStream()); + invokeCloseStream(); +} + } // namespace Config } // namespace Envoy diff --git a/test/extensions/config_subscription/grpc/grpc_mux_impl_test.cc b/test/extensions/config_subscription/grpc/grpc_mux_impl_test.cc index df541cd6bf..5108a0502a 100644 --- a/test/extensions/config_subscription/grpc/grpc_mux_impl_test.cc +++ b/test/extensions/config_subscription/grpc/grpc_mux_impl_test.cc @@ -27,6 +27,7 @@ #include "test/test_common/logging.h" #include "test/test_common/resources.h" #include "test/test_common/simulated_time_system.h" +#include "test/test_common/status_utility.h" #include "test/test_common/test_runtime.h" #include "test/test_common/test_time.h" #include "test/test_common/utility.h" @@ -95,7 +96,8 @@ class GrpcMuxImplTestBase : public testing::TestWithParam { const std::vector& resource_names, const std::string& version, bool first = false, const std::string& nonce = "", const Protobuf::int32 error_code = Grpc::Status::WellKnownGrpcStatus::Ok, - const std::string& error_message = "") { + const std::string& error_message = "", + Grpc::MockAsyncStream* async_stream = nullptr) { envoy::service::discovery::v3::DiscoveryRequest expected_request; if (first) { expected_request.mutable_node()->CopyFrom(local_info_.node()); @@ -113,7 +115,8 @@ class GrpcMuxImplTestBase : public testing::TestWithParam { error_detail->set_code(error_code); error_detail->set_message(error_message); } - EXPECT_CALL(async_stream_, sendMessageRaw_(Grpc::ProtoBufferEq(expected_request), false)); + EXPECT_CALL(async_stream ? *async_stream : async_stream_, + sendMessageRaw_(Grpc::ProtoBufferEq(expected_request), false)); } TestScopedRuntime scoped_runtime_; @@ -122,6 +125,9 @@ class GrpcMuxImplTestBase : public testing::TestWithParam { NiceMock local_info_; Grpc::MockAsyncClient* async_client_; Grpc::MockAsyncStream async_stream_; + // Used for tests invoking updateMuxSource(). + Grpc::MockAsyncClient* replaced_async_client_; + Grpc::MockAsyncStream replaced_async_stream_; CustomConfigValidatorsPtr config_validators_; GrpcMuxImplPtr grpc_mux_; NiceMock callbacks_; @@ -1346,6 +1352,172 @@ TEST_P(GrpcMuxImplTest, RemoveCachedResourceOnLastSubscription) { EXPECT_CALL(*eds_resources_cache_, removeResource("x")); } +// Updating the mux object while being connected sends the correct requests. +TEST_P(GrpcMuxImplTest, MuxDynamicReplacementWhenConnected) { + replaced_async_client_ = new Grpc::MockAsyncClient(); + setup(); + InSequence s; + + auto foo_sub = grpc_mux_->addWatch("type_url_foo", {"x", "y"}, callbacks_, resource_decoder_, {}); + auto bar_sub = grpc_mux_->addWatch("type_url_bar", {}, callbacks_, resource_decoder_, {}); + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); + expectSendMessage("type_url_foo", {"x", "y"}, "", true); + expectSendMessage("type_url_bar", {}, ""); + grpc_mux_->start(); + EXPECT_EQ(1, control_plane_connected_state_.value()); + + // Switch the mux. + envoy::config::core::v3::ApiConfigSource empty_ads_config; + // Expect a disconnect from the original async_client and stream. + EXPECT_CALL(async_stream_, resetStream()); + // Expect establishing connection to the new client and stream. + EXPECT_CALL(*replaced_async_client_, startRaw(_, _, _, _)) + .WillOnce(Return(&replaced_async_stream_)); + // Expect the initial messages to be sent to the new stream. + expectSendMessage("type_url_foo", {"x", "y"}, "", true, "", Grpc::Status::WellKnownGrpcStatus::Ok, + "", &replaced_async_stream_); + expectSendMessage("type_url_bar", {}, "", false, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + &replaced_async_stream_); + EXPECT_OK(grpc_mux_->updateMuxSource( + /*primary_async_client=*/std::unique_ptr(replaced_async_client_), + /*failover_async_client=*/nullptr, + /*custom_config_validators=*/nullptr, + /*scope=*/*stats_.rootScope(), + /*backoff_strategy=*/ + std::make_unique( + SubscriptionFactory::RetryInitialDelayMs, SubscriptionFactory::RetryMaxDelayMs, random_), + empty_ads_config)); + // Ending test, removing subscriptions for type_url_foo. + expectSendMessage("type_url_foo", {}, "", false, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + &replaced_async_stream_); +} + +// Updating the mux object after receiving a response, sends the correct requests. +TEST_P(GrpcMuxImplTest, MuxDynamicReplacementFetchingResources) { + replaced_async_client_ = new Grpc::MockAsyncClient(); + setup(); + InSequence s; + + const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment; + OpaqueResourceDecoderSharedPtr resource_decoder( + std::make_shared>("cluster_name")); + auto foo_sub = grpc_mux_->addWatch(type_url, {"x", "y"}, callbacks_, resource_decoder, {}); + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); + expectSendMessage(type_url, {"x", "y"}, "", true); + grpc_mux_->start(); + + // Send back a response for one of the resources. + { + auto response = std::make_unique(); + response->set_type_url(type_url); + response->set_version_info("1"); + envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment; + load_assignment.set_cluster_name("x"); + response->add_resources()->PackFrom(load_assignment); + EXPECT_CALL(callbacks_, onConfigUpdate(_, "1")) + .WillOnce(Invoke([&load_assignment](const std::vector& resources, + const std::string&) { + EXPECT_EQ(1, resources.size()); + const auto& expected_assignment = + dynamic_cast( + resources[0].get().resource()); + EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); + return absl::OkStatus(); + })); + expectSendMessage(type_url, {"x", "y"}, "1"); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); + } + + // Switch the mux. + envoy::config::core::v3::ApiConfigSource empty_ads_config; + // Expect a disconnect from the original async_client and stream. + EXPECT_CALL(async_stream_, resetStream()); + // Expect establishing connection to the new client and stream. + EXPECT_CALL(*replaced_async_client_, startRaw(_, _, _, _)) + .WillOnce(Return(&replaced_async_stream_)); + // Expect the initial message to be sent to the new stream. + expectSendMessage(type_url, {"x", "y"}, "1", true, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + &replaced_async_stream_); + EXPECT_OK(grpc_mux_->updateMuxSource( + /*primary_async_client=*/std::unique_ptr(replaced_async_client_), + /*failover_async_client=*/nullptr, + /*custom_config_validators=*/std::make_unique>(), + /*scope=*/*stats_.rootScope(), + /*backoff_strategy=*/ + std::make_unique( + SubscriptionFactory::RetryInitialDelayMs, SubscriptionFactory::RetryMaxDelayMs, random_), + empty_ads_config)); + + // Send a response to resource "y" on the replaced mux. + { + auto response = std::make_unique(); + response->set_type_url(type_url); + response->set_version_info("2"); + envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment; + load_assignment.set_cluster_name("y"); + response->add_resources()->PackFrom(load_assignment); + EXPECT_CALL(callbacks_, onConfigUpdate(_, "2")) + .WillOnce(Invoke([&load_assignment](const std::vector& resources, + const std::string&) { + EXPECT_EQ(1, resources.size()); + const auto& expected_assignment = + dynamic_cast( + resources[0].get().resource()); + EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); + return absl::OkStatus(); + })); + expectSendMessage(type_url, {"x", "y"}, "2", false, "", Grpc::Status::WellKnownGrpcStatus::Ok, + "", &replaced_async_stream_); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); + } + + // Ending test, removing subscriptions for the subscription. + expectSendMessage(type_url, {}, "2", false, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + &replaced_async_stream_); +} + +// Updating the mux object with wrong rate limit settings is rejected. +TEST_P(GrpcMuxImplTest, RejectMuxDynamicReplacementRateLimitSettingsError) { + replaced_async_client_ = new Grpc::MockAsyncClient(); + setup(); + InSequence s; + + auto foo_sub = grpc_mux_->addWatch("type_url_foo", {"x", "y"}, callbacks_, resource_decoder_, {}); + auto bar_sub = grpc_mux_->addWatch("type_url_bar", {}, callbacks_, resource_decoder_, {}); + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); + expectSendMessage("type_url_foo", {"x", "y"}, "", true); + expectSendMessage("type_url_bar", {}, ""); + grpc_mux_->start(); + EXPECT_EQ(1, control_plane_connected_state_.value()); + + // Switch the mux. + envoy::config::core::v3::ApiConfigSource ads_config_wrong_settings; + envoy::config::core::v3::RateLimitSettings* rate_limits = + ads_config_wrong_settings.mutable_rate_limit_settings(); + rate_limits->mutable_max_tokens()->set_value(500); + rate_limits->mutable_fill_rate()->set_value(std::numeric_limits::quiet_NaN()); + // No disconnect and replacement of the original async_client. + EXPECT_CALL(async_stream_, resetStream()).Times(0); + EXPECT_CALL(*replaced_async_client_, startRaw(_, _, _, _)).Times(0); + EXPECT_FALSE(grpc_mux_ + ->updateMuxSource( + /*primary_async_client=*/std::unique_ptr( + replaced_async_client_), + /*failover_async_client=*/nullptr, + /*custom_config_validators=*/nullptr, + /*scope=*/*stats_.rootScope(), + /*backoff_strategy=*/ + std::make_unique( + SubscriptionFactory::RetryInitialDelayMs, + SubscriptionFactory::RetryMaxDelayMs, random_), + ads_config_wrong_settings) + .ok()); + // Ending test, removing subscriptions for type_url_foo. + expectSendMessage("type_url_foo", {}, "", false, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + &async_stream_); +} + /** * Tests the NullGrpcMuxImpl object to increase code-coverage. */ @@ -1399,6 +1571,14 @@ TEST_F(NullGrpcMuxImplTest, OnDiscoveryResponseImplemented) { EXPECT_NO_THROW(null_mux_.onDiscoveryResponse(std::move(response), cp_stats)); } +TEST_F(NullGrpcMuxImplTest, UpdateMuxSourceError) { + Stats::TestUtil::TestStore stats; + const envoy::config::core::v3::ApiConfigSource empty_config; + const absl::Status status = null_mux_.updateMuxSource(nullptr, nullptr, nullptr, + *stats.rootScope(), nullptr, empty_config); + EXPECT_EQ(status.code(), absl::StatusCode::kUnimplemented); +} + TEST(GrpcMuxFactoryTest, InvalidRateLimit) { auto* factory = Config::Utility::getFactoryByName("envoy.config_mux.grpc_mux_factory"); diff --git a/test/extensions/config_subscription/grpc/new_grpc_mux_impl_test.cc b/test/extensions/config_subscription/grpc/new_grpc_mux_impl_test.cc index 5c27473e03..b787557025 100644 --- a/test/extensions/config_subscription/grpc/new_grpc_mux_impl_test.cc +++ b/test/extensions/config_subscription/grpc/new_grpc_mux_impl_test.cc @@ -28,6 +28,7 @@ #include "test/test_common/logging.h" #include "test/test_common/resources.h" #include "test/test_common/simulated_time_system.h" +#include "test/test_common/status_utility.h" #include "test/test_common/test_runtime.h" #include "test/test_common/test_time.h" #include "test/test_common/utility.h" @@ -101,7 +102,8 @@ class NewGrpcMuxImplTestBase : public testing::TestWithParam& initial_resource_versions = {}) { + const std::map& initial_resource_versions = {}, + Grpc::MockAsyncStream* async_stream = nullptr) { API_NO_BOOST(envoy::service::discovery::v3::DeltaDiscoveryRequest) expected_request; expected_request.mutable_node()->CopyFrom(local_info_.node()); for (const auto& resource : resource_names_subscribe) { @@ -120,7 +122,8 @@ class NewGrpcMuxImplTestBase : public testing::TestWithParamset_code(error_code); error_detail->set_message(error_message); } - EXPECT_CALL(async_stream_, sendMessageRaw_(Grpc::ProtoBufferEq(expected_request), false)); + EXPECT_CALL(async_stream ? *async_stream : async_stream_, + sendMessageRaw_(Grpc::ProtoBufferEq(expected_request), false)); } void remoteClose() { @@ -176,6 +179,9 @@ class NewGrpcMuxImplTestBase : public testing::TestWithParam random_; Grpc::MockAsyncClient* async_client_; NiceMock async_stream_; + // Used for tests invoking updateMuxSource(). + Grpc::MockAsyncClient* replaced_async_client_; + Grpc::MockAsyncStream replaced_async_stream_; CustomConfigValidatorsPtr config_validators_; NiceMock local_info_; std::unique_ptr grpc_mux_; @@ -795,6 +801,175 @@ TEST_P(NewGrpcMuxImplTest, AddRemoveSubscriptions) { } } +// Updating the mux object while being connected sends the correct requests. +TEST_P(NewGrpcMuxImplTest, MuxDynamicReplacementWhenConnected) { + replaced_async_client_ = new Grpc::MockAsyncClient(); + setup(); + InSequence s; + + auto foo_sub = grpc_mux_->addWatch("type_url_foo", {"x", "y"}, callbacks_, resource_decoder_, {}); + auto bar_sub = grpc_mux_->addWatch("type_url_bar", {}, callbacks_, resource_decoder_, {}); + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); + expectSendMessage("type_url_foo", {"x", "y"}, {}); + expectSendMessage("type_url_bar", {}, {}); + grpc_mux_->start(); + EXPECT_EQ(1, control_plane_connected_state_.value()); + + // Switch the mux. + envoy::config::core::v3::ApiConfigSource empty_ads_config; + // Expect a disconnect from the original async_client and stream. + EXPECT_CALL(async_stream_, resetStream()); + // Expect establishing connection to the new client and stream. + EXPECT_CALL(*replaced_async_client_, startRaw(_, _, _, _)) + .WillOnce(Return(&replaced_async_stream_)); + // Expect the initial messages to be sent to the new stream. + expectSendMessage("type_url_foo", {"x", "y"}, {}, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + {}, &replaced_async_stream_); + expectSendMessage("type_url_bar", {}, {}, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", {}, + &replaced_async_stream_); + EXPECT_OK(grpc_mux_->updateMuxSource( + /*primary_async_client=*/std::unique_ptr(replaced_async_client_), + /*failover_async_client=*/nullptr, + /*custom_config_validators=*/nullptr, + /*scope=*/*stats_.rootScope(), + /*backoff_strategy=*/ + std::make_unique( + SubscriptionFactory::RetryInitialDelayMs, SubscriptionFactory::RetryMaxDelayMs, random_), + empty_ads_config)); + // Ending test, removing subscriptions for type_url_foo. + expectSendMessage("type_url_foo", {}, {"x", "y"}, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + {}, &replaced_async_stream_); +} + +// Updating the mux object after receiving a response, sends the correct requests. +TEST_P(NewGrpcMuxImplTest, MuxDynamicReplacementFetchingResources) { + replaced_async_client_ = new Grpc::MockAsyncClient(); + setup(); + InSequence s; + + const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment; + auto foo_sub = grpc_mux_->addWatch(type_url, {"x", "y"}, callbacks_, resource_decoder_, {}); + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); + expectSendMessage(type_url, {"x", "y"}, {}); + grpc_mux_->start(); + + // Send back a response for one of the resources. + { + auto response = std::make_unique(); + response->set_type_url(type_url); + response->set_system_version_info("1"); + response->set_nonce("n1"); + envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment; + load_assignment.set_cluster_name("x"); + auto* resource = response->add_resources(); + resource->set_name("x"); + resource->mutable_resource()->PackFrom(load_assignment); + resource->set_version("x1"); + EXPECT_CALL(callbacks_, onConfigUpdate(_, _, "1")) + .WillOnce(Invoke([&load_assignment](const std::vector& added_resources, + const Protobuf::RepeatedPtrField&, + const std::string&) { + EXPECT_EQ(1, added_resources.size()); + EXPECT_TRUE( + TestUtility::protoEqual(added_resources[0].get().resource(), load_assignment)); + return absl::OkStatus(); + })); + expectSendMessage(type_url, {}, {}, "n1"); + onDiscoveryResponse(std::move(response)); + } + + // Switch the mux. + envoy::config::core::v3::ApiConfigSource empty_ads_config; + // Expect a disconnect from the original async_client and stream. + EXPECT_CALL(async_stream_, resetStream()); + // Expect establishing connection to the new client and stream. + EXPECT_CALL(*replaced_async_client_, startRaw(_, _, _, _)) + .WillOnce(Return(&replaced_async_stream_)); + // Expect the initial message to be sent to the new stream. + // It will include "x" in its initial_resource_versions. + expectSendMessage(type_url, {"x", "y"}, {}, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + {{"x", "x1"}}, &replaced_async_stream_); + EXPECT_OK(grpc_mux_->updateMuxSource( + /*primary_async_client=*/std::unique_ptr(replaced_async_client_), + /*failover_async_client=*/nullptr, + /*custom_config_validators=*/std::make_unique>(), + /*scope=*/*stats_.rootScope(), + /*backoff_strategy=*/ + std::make_unique( + SubscriptionFactory::RetryInitialDelayMs, SubscriptionFactory::RetryMaxDelayMs, random_), + empty_ads_config)); + + // Send a response to resource "y" on the replaced mux. + { + auto response = std::make_unique(); + response->set_type_url(type_url); + response->set_system_version_info("2"); + response->set_nonce("n2"); + envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment; + load_assignment.set_cluster_name("y"); + auto* resource = response->add_resources(); + resource->set_name("y"); + resource->mutable_resource()->PackFrom(load_assignment); + resource->set_version("y1"); + EXPECT_CALL(callbacks_, onConfigUpdate(_, _, "2")) + .WillOnce(Invoke([&load_assignment](const std::vector& added_resources, + const Protobuf::RepeatedPtrField&, + const std::string&) { + EXPECT_EQ(1, added_resources.size()); + EXPECT_TRUE( + TestUtility::protoEqual(added_resources[0].get().resource(), load_assignment)); + return absl::OkStatus(); + })); + expectSendMessage(type_url, {}, {}, "n2", Grpc::Status::WellKnownGrpcStatus::Ok, "", {}, + &replaced_async_stream_); + onDiscoveryResponse(std::move(response)); + } + + // Ending test, removing subscriptions for the subscription. + expectSendMessage(type_url, {}, {"x", "y"}, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", {}, + &replaced_async_stream_); +} + +// Updating the mux object with wrong rate limit settings is rejected. +TEST_P(NewGrpcMuxImplTest, RejectMuxDynamicReplacementRateLimitSettingsError) { + replaced_async_client_ = new Grpc::MockAsyncClient(); + setup(); + InSequence s; + + const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment; + auto foo_sub = grpc_mux_->addWatch(type_url, {"x", "y"}, callbacks_, resource_decoder_, {}); + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); + expectSendMessage(type_url, {"x", "y"}, {}); + grpc_mux_->start(); + EXPECT_EQ(1, control_plane_connected_state_.value()); + + // Switch the mux. + envoy::config::core::v3::ApiConfigSource ads_config_wrong_settings; + envoy::config::core::v3::RateLimitSettings* rate_limits = + ads_config_wrong_settings.mutable_rate_limit_settings(); + rate_limits->mutable_max_tokens()->set_value(500); + rate_limits->mutable_fill_rate()->set_value(std::numeric_limits::quiet_NaN()); + // No disconnect and replacement of the original async_client. + EXPECT_CALL(async_stream_, resetStream()).Times(0); + EXPECT_CALL(*replaced_async_client_, startRaw(_, _, _, _)).Times(0); + EXPECT_FALSE(grpc_mux_ + ->updateMuxSource( + /*primary_async_client=*/std::unique_ptr( + replaced_async_client_), + /*failover_async_client=*/nullptr, + /*custom_config_validators=*/nullptr, + /*scope=*/*stats_.rootScope(), + /*backoff_strategy=*/ + std::make_unique( + SubscriptionFactory::RetryInitialDelayMs, + SubscriptionFactory::RetryMaxDelayMs, random_), + ads_config_wrong_settings) + .ok()); + // Ending test, removing subscriptions for type_url_foo. + expectSendMessage(type_url, {}, {"x", "y"}, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", {}, + &async_stream_); +} + TEST(NewGrpcMuxFactoryTest, InvalidRateLimit) { auto* factory = Config::Utility::getFactoryByName( "envoy.config_mux.new_grpc_mux_factory"); diff --git a/test/extensions/config_subscription/grpc/watch_map_test.cc b/test/extensions/config_subscription/grpc/watch_map_test.cc index 4186b3d8b8..014a1221b0 100644 --- a/test/extensions/config_subscription/grpc/watch_map_test.cc +++ b/test/extensions/config_subscription/grpc/watch_map_test.cc @@ -133,7 +133,7 @@ TEST(WatchMapTest, Basic) { TestUtility::TestOpaqueResourceDecoderImpl resource_decoder("cluster_name"); NiceMock config_validators; - WatchMap watch_map(false, "ClusterLoadAssignmentType", config_validators, {}); + WatchMap watch_map(false, "ClusterLoadAssignmentType", &config_validators, {}); Watch* watch = watch_map.addWatch(callbacks, resource_decoder); { @@ -207,7 +207,7 @@ TEST(WatchMapTest, Overlap) { TestUtility::TestOpaqueResourceDecoderImpl resource_decoder("cluster_name"); NiceMock config_validators; - WatchMap watch_map(false, "ClusterLoadAssignmentType", config_validators, {}); + WatchMap watch_map(false, "ClusterLoadAssignmentType", &config_validators, {}); Watch* watch1 = watch_map.addWatch(callbacks1, resource_decoder); Watch* watch2 = watch_map.addWatch(callbacks2, resource_decoder); @@ -276,7 +276,7 @@ TEST(WatchMapTest, CacheResourceAddResource) { NiceMock eds_resources_cache; const std::string eds_type_url = Config::getTypeUrl(); - WatchMap watch_map(false, eds_type_url, config_validators, + WatchMap watch_map(false, eds_type_url, &config_validators, makeOptRef(eds_resources_cache)); // The test uses 2 watchers to ensure that interest is kept regardless of // which watcher was the first to add a watch for the assignment. @@ -357,7 +357,7 @@ TEST(WatchMapTest, CacheResourceAddResource) { // WatchMap defers deletes and doesn't crash. class SameWatchRemoval : public testing::Test { public: - SameWatchRemoval() : watch_map_(false, "ClusterLoadAssignmentType", config_validators, {}) {} + SameWatchRemoval() : watch_map_(false, "ClusterLoadAssignmentType", &config_validators, {}) {} void SetUp() override { envoy::config::endpoint::v3::ClusterLoadAssignment alice; @@ -437,7 +437,7 @@ TEST(WatchMapTest, AddRemoveAdd) { TestUtility::TestOpaqueResourceDecoderImpl resource_decoder("cluster_name"); NiceMock config_validators; - WatchMap watch_map(false, "ClusterLoadAssignmentType", config_validators, {}); + WatchMap watch_map(false, "ClusterLoadAssignmentType", &config_validators, {}); Watch* watch1 = watch_map.addWatch(callbacks1, resource_decoder); Watch* watch2 = watch_map.addWatch(callbacks2, resource_decoder); @@ -494,7 +494,7 @@ TEST(WatchMapTest, UninterestingUpdate) { TestUtility::TestOpaqueResourceDecoderImpl resource_decoder("cluster_name"); NiceMock config_validators; - WatchMap watch_map(false, "ClusterLoadAssignmentType", config_validators, {}); + WatchMap watch_map(false, "ClusterLoadAssignmentType", &config_validators, {}); Watch* watch = watch_map.addWatch(callbacks, resource_decoder); watch_map.updateWatchInterest(watch, {"alice"}); @@ -539,7 +539,7 @@ TEST(WatchMapTest, WatchingEverything) { TestUtility::TestOpaqueResourceDecoderImpl resource_decoder("cluster_name"); NiceMock config_validators; - WatchMap watch_map(false, "ClusterLoadAssignmentType", config_validators, {}); + WatchMap watch_map(false, "ClusterLoadAssignmentType", &config_validators, {}); /*Watch* watch1 = */ watch_map.addWatch(callbacks1, resource_decoder); Watch* watch2 = watch_map.addWatch(callbacks2, resource_decoder); // watch1 never specifies any names, and so is treated as interested in everything. @@ -576,7 +576,7 @@ TEST(WatchMapTest, DeltaOnConfigUpdate) { TestUtility::TestOpaqueResourceDecoderImpl resource_decoder("cluster_name"); NiceMock config_validators; - WatchMap watch_map(false, "ClusterLoadAssignmentType", config_validators, {}); + WatchMap watch_map(false, "ClusterLoadAssignmentType", &config_validators, {}); Watch* watch1 = watch_map.addWatch(callbacks1, resource_decoder); Watch* watch2 = watch_map.addWatch(callbacks2, resource_decoder); Watch* watch3 = watch_map.addWatch(callbacks3, resource_decoder); @@ -610,7 +610,7 @@ TEST(WatchMapTest, DeltaOnConfigUpdate) { TEST(WatchMapTest, OnConfigUpdateFailed) { NiceMock config_validators; - WatchMap watch_map(false, "ClusterLoadAssignmentType", config_validators, {}); + WatchMap watch_map(false, "ClusterLoadAssignmentType", &config_validators, {}); // calling on empty map doesn't break watch_map.onConfigUpdateFailed(ConfigUpdateFailureReason::UpdateRejected, nullptr); @@ -632,7 +632,7 @@ TEST(WatchMapTest, OnConfigUpdateXdsTpGlobCollections) { TestUtility::TestOpaqueResourceDecoderImpl resource_decoder("cluster_name"); NiceMock config_validators; - WatchMap watch_map(false, "ClusterLoadAssignmentType", config_validators, {}); + WatchMap watch_map(false, "ClusterLoadAssignmentType", &config_validators, {}); Watch* watch = watch_map.addWatch(callbacks, resource_decoder); watch_map.updateWatchInterest(watch, {"xdstp://foo/bar/baz/*?some=thing&thing=some"}); @@ -677,7 +677,7 @@ TEST(WatchMapTest, OnConfigUpdateXdsTpSingletons) { TestUtility::TestOpaqueResourceDecoderImpl resource_decoder("cluster_name"); NiceMock config_validators; - WatchMap watch_map(false, "ClusterLoadAssignmentType", config_validators, {}); + WatchMap watch_map(false, "ClusterLoadAssignmentType", &config_validators, {}); Watch* watch = watch_map.addWatch(callbacks, resource_decoder); watch_map.updateWatchInterest(watch, {"xdstp://foo/bar/baz?some=thing&thing=some"}); @@ -718,7 +718,7 @@ TEST(WatchMapTest, OnConfigUpdateUsingNamespaces) { TestUtility::TestOpaqueResourceDecoderImpl resource_decoder("cluster_name"); NiceMock config_validators; - WatchMap watch_map(true, "ClusterLoadAssignmentType", config_validators, {}); + WatchMap watch_map(true, "ClusterLoadAssignmentType", &config_validators, {}); Watch* watch1 = watch_map.addWatch(callbacks1, resource_decoder); Watch* watch2 = watch_map.addWatch(callbacks2, resource_decoder); Watch* watch3 = watch_map.addWatch(callbacks3, resource_decoder); diff --git a/test/extensions/config_subscription/grpc/xds_grpc_mux_impl_test.cc b/test/extensions/config_subscription/grpc/xds_grpc_mux_impl_test.cc index 123ffd52a5..f81b1da3ff 100644 --- a/test/extensions/config_subscription/grpc/xds_grpc_mux_impl_test.cc +++ b/test/extensions/config_subscription/grpc/xds_grpc_mux_impl_test.cc @@ -26,6 +26,7 @@ #include "test/test_common/logging.h" #include "test/test_common/resources.h" #include "test/test_common/simulated_time_system.h" +#include "test/test_common/status_utility.h" #include "test/test_common/test_runtime.h" #include "test/test_common/test_time.h" #include "test/test_common/utility.h" @@ -94,7 +95,8 @@ class GrpcMuxImplTestBase : public testing::TestWithParam { const std::vector& resource_names, const std::string& version, bool first = false, const std::string& nonce = "", const Protobuf::int32 error_code = Grpc::Status::WellKnownGrpcStatus::Ok, - const std::string& error_message = "") { + const std::string& error_message = "", + Grpc::MockAsyncStream* async_stream = nullptr) { envoy::service::discovery::v3::DiscoveryRequest expected_request; if (first) { expected_request.mutable_node()->CopyFrom(local_info_.node()); @@ -113,7 +115,7 @@ class GrpcMuxImplTestBase : public testing::TestWithParam { error_detail->set_message(error_message); } EXPECT_CALL( - async_stream_, + async_stream ? *async_stream : async_stream_, sendMessageRaw_(Grpc::ProtoBufferEqIgnoreRepeatedFieldOrdering(expected_request), false)); } @@ -134,6 +136,9 @@ class GrpcMuxImplTestBase : public testing::TestWithParam { NiceMock random_; Grpc::MockAsyncClient* async_client_; Grpc::MockAsyncStream async_stream_; + // Used for tests invoking updateMuxSource(). + Grpc::MockAsyncClient* replaced_async_client_; + Grpc::MockAsyncStream replaced_async_stream_; NiceMock local_info_; CustomConfigValidatorsPtr config_validators_; std::unique_ptr grpc_mux_; @@ -1279,6 +1284,168 @@ TEST_P(GrpcMuxImplTest, AddRemoveSubscriptions) { } } +// Updating the mux object while being connected sends the correct requests. +TEST_P(GrpcMuxImplTest, MuxDynamicReplacementWhenConnected) { + replaced_async_client_ = new Grpc::MockAsyncClient(); + setup(); + InSequence s; + + auto foo_sub = makeWatch("type_url_foo", {"x", "y"}); + auto bar_sub = makeWatch("type_url_bar", {}); + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); + expectSendMessage("type_url_foo", {"x", "y"}, "", true); + expectSendMessage("type_url_bar", {}, ""); + grpc_mux_->start(); + EXPECT_EQ(1, control_plane_connected_state_.value()); + + // Switch the mux. + envoy::config::core::v3::ApiConfigSource empty_ads_config; + // Expect a disconnect from the original async_client and stream. + EXPECT_CALL(async_stream_, resetStream()); + // Expect establishing connection to the new client and stream. + EXPECT_CALL(*replaced_async_client_, startRaw(_, _, _, _)) + .WillOnce(Return(&replaced_async_stream_)); + // Expect the initial messages to be sent to the new stream. + expectSendMessage("type_url_foo", {"x", "y"}, "", true, "", Grpc::Status::WellKnownGrpcStatus::Ok, + "", &replaced_async_stream_); + expectSendMessage("type_url_bar", {}, "", false, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + &replaced_async_stream_); + EXPECT_OK(grpc_mux_->updateMuxSource( + /*primary_async_client=*/std::unique_ptr(replaced_async_client_), + /*failover_async_client=*/nullptr, + /*custom_config_validators=*/nullptr, + /*scope=*/*stats_.rootScope(), + /*backoff_strategy=*/ + std::make_unique( + SubscriptionFactory::RetryInitialDelayMs, SubscriptionFactory::RetryMaxDelayMs, random_), + empty_ads_config)); + // Ending test, removing subscriptions for type_url_foo. + expectSendMessage("type_url_foo", {}, "", false, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + &replaced_async_stream_); +} + +// Updating the mux object after receiving a response, sends the correct requests. +TEST_P(GrpcMuxImplTest, MuxDynamicReplacementFetchingResources) { + replaced_async_client_ = new Grpc::MockAsyncClient(); + setup(); + InSequence s; + + const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment; + auto foo_sub = makeWatch(type_url, {"x", "y"}); + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); + expectSendMessage(type_url, {"x", "y"}, "", true); + grpc_mux_->start(); + + // Send back a response for one of the resources. + { + auto response = std::make_unique(); + response->set_type_url(type_url); + response->set_version_info("1"); + envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment; + load_assignment.set_cluster_name("x"); + response->add_resources()->PackFrom(load_assignment); + EXPECT_CALL(callbacks_, onConfigUpdate(_, "1")) + .WillOnce(Invoke([&load_assignment](const std::vector& resources, + const std::string&) { + EXPECT_EQ(1, resources.size()); + const auto& expected_assignment = + dynamic_cast( + resources[0].get().resource()); + EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); + return absl::OkStatus(); + })); + expectSendMessage(type_url, {"x", "y"}, "1"); + grpc_mux_->onDiscoveryResponse(std::move(response), control_plane_stats_); + } + + // Switch the mux. + envoy::config::core::v3::ApiConfigSource empty_ads_config; + // Expect a disconnect from the original async_client and stream. + EXPECT_CALL(async_stream_, resetStream()); + // Expect establishing connection to the new client and stream. + EXPECT_CALL(*replaced_async_client_, startRaw(_, _, _, _)) + .WillOnce(Return(&replaced_async_stream_)); + // Expect the initial message to be sent to the new stream. + expectSendMessage(type_url, {"x", "y"}, "1", true, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + &replaced_async_stream_); + EXPECT_OK(grpc_mux_->updateMuxSource( + /*primary_async_client=*/std::unique_ptr(replaced_async_client_), + /*failover_async_client=*/nullptr, + /*custom_config_validators=*/std::make_unique>(), + /*scope=*/*stats_.rootScope(), + /*backoff_strategy=*/ + std::make_unique( + SubscriptionFactory::RetryInitialDelayMs, SubscriptionFactory::RetryMaxDelayMs, random_), + empty_ads_config)); + + // Send a response to resource "y" on the replaced mux. + { + auto response = std::make_unique(); + response->set_type_url(type_url); + response->set_version_info("2"); + envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment; + load_assignment.set_cluster_name("y"); + response->add_resources()->PackFrom(load_assignment); + EXPECT_CALL(callbacks_, onConfigUpdate(_, "2")) + .WillOnce(Invoke([&load_assignment](const std::vector& resources, + const std::string&) { + EXPECT_EQ(1, resources.size()); + const auto& expected_assignment = + dynamic_cast( + resources[0].get().resource()); + EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); + return absl::OkStatus(); + })); + expectSendMessage(type_url, {"x", "y"}, "2", false, "", Grpc::Status::WellKnownGrpcStatus::Ok, + "", &replaced_async_stream_); + grpc_mux_->onDiscoveryResponse(std::move(response), control_plane_stats_); + } + + // Ending test, removing subscriptions for the subscription. + expectSendMessage(type_url, {}, "2", false, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + &replaced_async_stream_); +} + +// Updating the mux object with wrong rate limit settings is rejected. +TEST_P(GrpcMuxImplTest, RejectMuxDynamicReplacementRateLimitSettingsError) { + replaced_async_client_ = new Grpc::MockAsyncClient(); + setup(); + InSequence s; + + const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment; + auto foo_sub = makeWatch(type_url, {"x", "y"}); + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); + expectSendMessage(type_url, {"x", "y"}, "", true); + grpc_mux_->start(); + EXPECT_EQ(1, control_plane_connected_state_.value()); + + // Switch the mux. + envoy::config::core::v3::ApiConfigSource ads_config_wrong_settings; + envoy::config::core::v3::RateLimitSettings* rate_limits = + ads_config_wrong_settings.mutable_rate_limit_settings(); + rate_limits->mutable_max_tokens()->set_value(500); + rate_limits->mutable_fill_rate()->set_value(std::numeric_limits::quiet_NaN()); + // No disconnect and replacement of the original async_client. + EXPECT_CALL(async_stream_, resetStream()).Times(0); + EXPECT_CALL(*replaced_async_client_, startRaw(_, _, _, _)).Times(0); + EXPECT_FALSE(grpc_mux_ + ->updateMuxSource( + /*primary_async_client=*/std::unique_ptr( + replaced_async_client_), + /*failover_async_client=*/nullptr, + /*custom_config_validators=*/nullptr, + /*scope=*/*stats_.rootScope(), + /*backoff_strategy=*/ + std::make_unique( + SubscriptionFactory::RetryInitialDelayMs, + SubscriptionFactory::RetryMaxDelayMs, random_), + ads_config_wrong_settings) + .ok()); + // Ending test, removing subscriptions for type_url_foo. + expectSendMessage(type_url, {}, "", false, "", Grpc::Status::WellKnownGrpcStatus::Ok, "", + &async_stream_); +} + class NullGrpcMuxImplTest : public testing::Test { public: NullGrpcMuxImplTest() : null_mux_(std::make_unique()) {} diff --git a/test/extensions/filters/http/ext_proc/client_test.cc b/test/extensions/filters/http/ext_proc/client_test.cc index 590cd2e05e..6e910c367c 100644 --- a/test/extensions/filters/http/ext_proc/client_test.cc +++ b/test/extensions/filters/http/ext_proc/client_test.cc @@ -66,6 +66,8 @@ class ExtProcStreamTest : public testing::Test, public ExternalProcessorCallback void onGrpcClose() override { grpc_closed_ = true; } void logGrpcStreamInfo() override {} + void onComplete(envoy::service::ext_proc::v3::ProcessingResponse&) override {} + void onError() override {} std::unique_ptr last_response_; Grpc::Status::GrpcStatus grpc_status_ = Grpc::Status::WellKnownGrpcStatus::Ok; diff --git a/test/extensions/filters/http/ext_proc/config_test.cc b/test/extensions/filters/http/ext_proc/config_test.cc index 5c5196af65..b635fc9e30 100644 --- a/test/extensions/filters/http/ext_proc/config_test.cc +++ b/test/extensions/filters/http/ext_proc/config_test.cc @@ -60,7 +60,7 @@ TEST(HttpExtProcConfigTest, CorrectConfig) { cb(filter_callback); } -TEST(HttpExtProcConfigTest, CorrectConfigServerContext) { +TEST(HttpExtProcConfigTest, CorrectGrpcServiceConfigServerContext) { std::string yaml = R"EOF( grpc_service: google_grpc: @@ -106,6 +106,33 @@ TEST(HttpExtProcConfigTest, CorrectConfigServerContext) { cb(filter_callback); } +TEST(HttpExtProcConfigTest, CorrectHttpServiceConfigServerContext) { + std::string yaml = R"EOF( + http_service: + http_service: + http_uri: + uri: "ext_proc_server_0:9000" + cluster: "ext_proc_server_0" + timeout: + seconds: 500 + failure_mode_allow: true + processing_mode: + request_header_mode: send + )EOF"; + + ExternalProcessingFilterConfig factory; + ProtobufTypes::MessagePtr proto_config = factory.createEmptyConfigProto(); + TestUtility::loadFromYaml(yaml, *proto_config); + + testing::NiceMock context; + EXPECT_CALL(context, messageValidationVisitor()); + Http::FilterFactoryCb cb = + factory.createFilterFactoryFromProtoWithServerContext(*proto_config, "stats", context); + Http::MockFilterChainFactoryCallbacks filter_callback; + EXPECT_CALL(filter_callback, addStreamFilter(_)); + cb(filter_callback); +} + TEST(HttpExtProcConfigTest, CorrectRouteMetadataOnlyConfig) { std::string yaml = R"EOF( overrides: diff --git a/test/extensions/filters/http/ext_proc/filter_test.cc b/test/extensions/filters/http/ext_proc/filter_test.cc index edb3c2b664..148d47e2de 100644 --- a/test/extensions/filters/http/ext_proc/filter_test.cc +++ b/test/extensions/filters/http/ext_proc/filter_test.cc @@ -135,7 +135,7 @@ class HttpFilterTest : public testing::Test { std::make_shared( Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)), factory_context_); - filter_ = std::make_unique(config_, std::move(client_), proto_config.grpc_service()); + filter_ = std::make_unique(config_, std::move(client_)); filter_->setEncoderFilterCallbacks(encoder_callbacks_); EXPECT_CALL(encoder_callbacks_, encoderBufferLimit()).WillRepeatedly(Return(BufferSize)); filter_->setDecoderFilterCallbacks(decoder_callbacks_); @@ -2750,6 +2750,93 @@ TEST_F(HttpFilterTest, ProcessingModeResponseHeadersOnlyWithoutCallingDecodeHead EXPECT_EQ(1, config_->stats().streams_closed_.value()); } +TEST_F(HttpFilterTest, GrpcServiceHttpServiceBothSet) { + std::string yaml = R"EOF( + grpc_service: + envoy_grpc: + cluster_name: "ext_proc_server" + http_service: + http_service: + http_uri: + uri: "ext_proc_server_0:9000" + cluster: "ext_proc_server_0" + timeout: + seconds: 500 + processing_mode: + response_body_mode: "BUFFERED" + )EOF"; + + envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor proto_config{}; + TestUtility::loadFromYaml(yaml, proto_config); + EXPECT_THROW_WITH_MESSAGE( + { + auto config = std::make_shared( + proto_config, 200ms, 10000, *stats_store_.rootScope(), "", false, + std::make_shared( + Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)), + factory_context_); + }, + EnvoyException, "One and only one of grpc_service or http_service must be configured"); +} + +TEST_F(HttpFilterTest, HttpServiceBodyProcessingModeNotNone) { + std::string yaml = R"EOF( + http_service: + http_service: + http_uri: + uri: "ext_proc_server_0:9000" + cluster: "ext_proc_server_0" + timeout: + seconds: 500 + processing_mode: + response_header_mode: "SEND" + request_body_mode: "BUFFERED" + )EOF"; + + envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor proto_config{}; + TestUtility::loadFromYaml(yaml, proto_config); + EXPECT_THROW_WITH_MESSAGE( + { + auto config = std::make_shared( + proto_config, 200ms, 10000, *stats_store_.rootScope(), "", false, + std::make_shared( + Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)), + factory_context_); + }, + EnvoyException, + "If http_service is configured, processing modes can not send any body or trailer."); +} + +TEST_F(HttpFilterTest, HttpServiceTrailerProcessingModeNotSKIP) { + std::string yaml = R"EOF( + http_service: + http_service: + http_uri: + uri: "ext_proc_server_0:9000" + cluster: "ext_proc_server_0" + timeout: + seconds: 500 + processing_mode: + request_body_mode: "NONE" + response_body_mode: "NONE" + request_trailer_mode: "SKIP" + response_trailer_mode: "SEND" + )EOF"; + + envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor proto_config{}; + TestUtility::loadFromYaml(yaml, proto_config); + EXPECT_THROW_WITH_MESSAGE( + { + auto config = std::make_shared( + proto_config, 200ms, 10000, *stats_store_.rootScope(), "", false, + std::make_shared( + Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)), + factory_context_); + }, + EnvoyException, + "If http_service is configured, processing modes can not send any body or trailer."); +} + // Using the default configuration, verify that the "clear_route_cache" flag makes the appropriate // callback on the filter for inbound traffic when header modifications are also present. // Also verify it does not make the callback for outbound traffic. diff --git a/test/extensions/filters/http/ext_proc/http_client/BUILD b/test/extensions/filters/http/ext_proc/http_client/BUILD index 0cc7d2bdab..1a073794cc 100644 --- a/test/extensions/filters/http/ext_proc/http_client/BUILD +++ b/test/extensions/filters/http/ext_proc/http_client/BUILD @@ -25,3 +25,24 @@ envoy_extension_cc_test( "@envoy_api//envoy/extensions/filters/http/ext_proc/v3:pkg_cc_proto", ], ) + +envoy_extension_cc_test( + name = "ext_proc_http_integration_test", + srcs = ["ext_proc_http_integration_test.cc"], + extension_names = ["envoy.filters.http.ext_proc"], + rbe_pool = "2core", + shard_count = 8, + tags = [ + "cpu:3", + "skip_on_windows", + ], + deps = [ + "//source/extensions/filters/http/ext_proc:config", + "//test/common/http:common_lib", + "//test/extensions/filters/http/ext_proc:utils_lib", + "//test/integration:http_protocol_integration_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/extensions/filters/http/ext_proc/v3:pkg_cc_proto", + "@envoy_api//envoy/service/ext_proc/v3:pkg_cc_proto", + ], +) diff --git a/test/extensions/filters/http/ext_proc/http_client/ext_proc_http_integration_test.cc b/test/extensions/filters/http/ext_proc/http_client/ext_proc_http_integration_test.cc new file mode 100644 index 0000000000..1b8e9899fc --- /dev/null +++ b/test/extensions/filters/http/ext_proc/http_client/ext_proc_http_integration_test.cc @@ -0,0 +1,493 @@ +#include +#include + +#include "envoy/extensions/filters/http/ext_proc/v3/ext_proc.pb.h" +#include "envoy/service/ext_proc/v3/external_processor.pb.h" + +#include "source/extensions/filters/http/ext_proc/config.h" +#include "source/extensions/filters/http/ext_proc/ext_proc.h" + +#include "test/common/http/common.h" +#include "test/extensions/filters/http/ext_proc/utils.h" +#include "test/integration/http_protocol_integration.h" +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace ExternalProcessing { +namespace { + +using envoy::extensions::filters::http::ext_proc::v3::ProcessingMode; +using envoy::service::ext_proc::v3::BodyResponse; +using envoy::service::ext_proc::v3::CommonResponse; +using envoy::service::ext_proc::v3::HeadersResponse; +using envoy::service::ext_proc::v3::HttpBody; +using envoy::service::ext_proc::v3::HttpHeaders; +using envoy::service::ext_proc::v3::HttpTrailers; +using envoy::service::ext_proc::v3::ProcessingRequest; +using envoy::service::ext_proc::v3::ProcessingResponse; +using envoy::service::ext_proc::v3::TrailersResponse; +using Extensions::HttpFilters::ExternalProcessing::HasHeader; +using Extensions::HttpFilters::ExternalProcessing::HasNoHeader; +using Extensions::HttpFilters::ExternalProcessing::HeaderProtosEqual; +using Extensions::HttpFilters::ExternalProcessing::SingleHeaderValueIs; + +using Http::LowerCaseString; + +struct ConfigOptions { + bool downstream_filter = true; + bool failure_mode_allow = false; + int64_t timeout = 900000000; + std::string cluster = "ext_proc_server_0"; +}; + +struct ExtProcHttpTestParams { + Network::Address::IpVersion version; + Http::CodecType downstream_protocol; + Http::CodecType upstream_protocol; +}; + +class ExtProcHttpClientIntegrationTest : public testing::TestWithParam, + public HttpIntegrationTest { +public: + ExtProcHttpClientIntegrationTest() + : HttpIntegrationTest(GetParam().downstream_protocol, GetParam().version) {} + void createUpstreams() override { + HttpIntegrationTest::createUpstreams(); + + // Create separate "upstreams" for ExtProc side stream servers + for (int i = 0; i < side_stream_count_; ++i) { + http_side_upstreams_.push_back(&addFakeUpstream(Http::CodecType::HTTP2)); + } + } + + void TearDown() override { + if (processor_connection_) { + ASSERT_TRUE(processor_connection_->close()); + ASSERT_TRUE(processor_connection_->waitForDisconnect()); + } + cleanupUpstreamAndDownstream(); + } + + void initializeConfig(ConfigOptions config_option = {}, + const std::vector>& cluster_endpoints = {{0, 1}, + {1, 1}}) { + int total_cluster_endpoints = 0; + std::for_each( + cluster_endpoints.begin(), cluster_endpoints.end(), + [&total_cluster_endpoints](const auto& item) { total_cluster_endpoints += item.second; }); + ASSERT_EQ(total_cluster_endpoints, side_stream_count_); + + config_helper_.addConfigModifier([this, cluster_endpoints, config_option]( + envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + // Ensure "HTTP2 with no prior knowledge." + ConfigHelper::setHttp2( + *(bootstrap.mutable_static_resources()->mutable_clusters()->Mutable(0))); + + // Clusters for ExtProc servers, starting by copying an existing cluster. + for (const auto& [cluster_id, endpoint_count] : cluster_endpoints) { + auto* server_cluster = bootstrap.mutable_static_resources()->add_clusters(); + server_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); + std::string cluster_name = absl::StrCat("ext_proc_server_", cluster_id); + server_cluster->set_name(cluster_name); + server_cluster->mutable_load_assignment()->set_cluster_name(cluster_name); + ASSERT_EQ(server_cluster->load_assignment().endpoints_size(), 1); + auto* endpoints = server_cluster->mutable_load_assignment()->mutable_endpoints(0); + ASSERT_EQ(endpoints->lb_endpoints_size(), 1); + for (int i = 1; i < endpoint_count; ++i) { + auto* new_lb_endpoint = endpoints->add_lb_endpoints(); + new_lb_endpoint->MergeFrom(endpoints->lb_endpoints(0)); + } + } + + auto* http_uri = + proto_config_.mutable_http_service()->mutable_http_service()->mutable_http_uri(); + http_uri->set_uri("ext_proc_server_0:9000"); + http_uri->set_cluster(config_option.cluster); + http_uri->mutable_timeout()->set_nanos(config_option.timeout); + + if (config_option.failure_mode_allow) { + proto_config_.set_failure_mode_allow(true); + } + std::string ext_proc_filter_name = "envoy.filters.http.ext_proc"; + if (config_option.downstream_filter) { + // Construct a configuration proto for our filter and then re-write it + // to JSON so that we can add it to the overall config + envoy::extensions::filters::network::http_connection_manager::v3::HttpFilter + ext_proc_filter; + ext_proc_filter.set_name(ext_proc_filter_name); + ext_proc_filter.mutable_typed_config()->PackFrom(proto_config_); + config_helper_.prependFilter(MessageUtil::getJsonStringFromMessageOrError(ext_proc_filter)); + } + }); + + setUpstreamProtocol(GetParam().upstream_protocol); + setDownstreamProtocol(GetParam().downstream_protocol); + } + + IntegrationStreamDecoderPtr sendDownstreamRequest( + absl::optional> modify_headers) { + auto conn = makeClientConnection(lookupPort("http")); + codec_client_ = makeHttpConnection(std::move(conn)); + Http::TestRequestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + if (modify_headers) { + (*modify_headers)(headers); + } + return codec_client_->makeHeaderOnlyRequest(headers); + } + + IntegrationStreamDecoderPtr sendDownstreamRequestWithBodyAndTrailer(absl::string_view body) { + codec_client_ = makeHttpConnection(lookupPort("http")); + Http::TestRequestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + + auto encoder_decoder = codec_client_->startRequest(headers); + request_encoder_ = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + codec_client_->sendData(*request_encoder_, body, false); + Http::TestRequestTrailerMapImpl request_trailers{{"x-trailer-foo", "yes"}}; + codec_client_->sendTrailers(*request_encoder_, request_trailers); + + return response; + } + + void getAndCheckHttpRequest(FakeUpstream* side_stream, bool first_message = false) { + if (first_message) { + ASSERT_TRUE(side_stream->waitForHttpConnection(*dispatcher_, processor_connection_)); + } + + ASSERT_TRUE(processor_connection_->waitForNewStream(*dispatcher_, processor_stream_)); + ASSERT_TRUE(processor_stream_->waitForEndStream(*dispatcher_)); + EXPECT_THAT(processor_stream_->headers(), + SingleHeaderValueIs("content-type", "application/json")); + EXPECT_THAT(processor_stream_->headers(), SingleHeaderValueIs(":method", "POST")); + EXPECT_THAT(processor_stream_->headers(), HasHeader("x-request-id")); + } + + void sendHttpResponse(ProcessingResponse& response) { + // Sending 200 response with the ProcessingResponse JSON encoded in the body. + std::string response_str = MessageUtil::getJsonStringFromMessageOrError(response, true, true); + processor_stream_->encodeHeaders( + Http::TestResponseHeaderMapImpl{{":status", "200"}, {"content-type", "application/json"}}, + false); + processor_stream_->encodeData(response_str, true); + } + + void processRequestHeadersMessage( + FakeUpstream* side_stream, bool first_message, + absl::optional> cb, + bool send_bad_resp = false) { + getAndCheckHttpRequest(side_stream, first_message); + + if (send_bad_resp) { + processor_stream_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "400"}}, true); + return; + } + // The ext_proc ProcessingRequest message is JSON encoded in the body of the HTTP message. + std::string body = processor_stream_->body().toString(); + ProcessingRequest request; + bool has_unknown_field; + auto status = MessageUtil::loadFromJsonNoThrow(body, request, has_unknown_field); + if (status.ok()) { + ProcessingResponse response; + auto* headers = response.mutable_request_headers(); + const bool sendReply = !cb || (*cb)(request.request_headers(), *headers); + if (sendReply) { + sendHttpResponse(response); + } + } else { + processor_stream_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "400"}}, true); + } + } + + void processResponseHeadersMessage( + FakeUpstream* side_stream, bool first_message, + absl::optional> cb) { + getAndCheckHttpRequest(side_stream, first_message); + + std::string body = processor_stream_->body().toString(); + ProcessingRequest request; + bool has_unknown_field; + auto status = MessageUtil::loadFromJsonNoThrow(body, request, has_unknown_field); + if (status.ok()) { + ProcessingResponse response; + auto* headers = response.mutable_response_headers(); + const bool sendReply = !cb || (*cb)(request.response_headers(), *headers); + if (sendReply) { + sendHttpResponse(response); + } + } else { + processor_stream_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "400"}}, true); + } + } + + void handleUpstreamRequest() { + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); + } + + void handleUpstreamRequestWithTrailer() { + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, false); + upstream_request_->encodeData(100, false); + upstream_request_->encodeTrailers(Http::TestResponseTrailerMapImpl{{"x-test-trailers", "Yes"}}); + } + + void verifyDownstreamResponse(IntegrationStreamDecoder& response, int status_code) { + ASSERT_TRUE(response.waitForEndStream()); + EXPECT_TRUE(response.complete()); + EXPECT_EQ(std::to_string(status_code), response.headers().getStatusValue()); + } + + static std::vector getValuesForExtProcHttpTest() { + std::vector ret; + for (auto ip_version : TestEnvironment::getIpVersionsForTest()) { + for (auto downstream_protocol : {Http::CodecType::HTTP1, Http::CodecType::HTTP2}) { + for (auto upstream_protocol : {Http::CodecType::HTTP1, Http::CodecType::HTTP2}) { + ExtProcHttpTestParams params; + params.version = ip_version; + params.downstream_protocol = downstream_protocol; + params.upstream_protocol = upstream_protocol; + ret.push_back(params); + } + } + } + return ret; + } + + static std::string + ExtProcHttpTestParamsToString(const ::testing::TestParamInfo& params) { + return absl::StrCat( + (params.param.version == Network::Address::IpVersion::v4 ? "IPv4_" : "IPv6_"), + (params.param.downstream_protocol == Http::CodecType::HTTP1 ? "HTTP1_DS_" : "HTTP2_DS_"), + (params.param.upstream_protocol == Http::CodecType::HTTP1 ? "HTTP1_US" : "HTTP2_US")); + } + + envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor proto_config_{}; + std::vector http_side_upstreams_; + FakeHttpConnectionPtr processor_connection_; + FakeStreamPtr processor_stream_; + // Number of side stream servers in the test. + int side_stream_count_ = 2; +}; + +INSTANTIATE_TEST_SUITE_P( + Protocols, ExtProcHttpClientIntegrationTest, + testing::ValuesIn(ExtProcHttpClientIntegrationTest::getValuesForExtProcHttpTest()), + ExtProcHttpClientIntegrationTest::ExtProcHttpTestParamsToString); + +// Side stream server does not mutate the request header. +TEST_P(ExtProcHttpClientIntegrationTest, ServerNoRequestHeaderMutation) { + proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SKIP); + + initializeConfig(); + HttpIntegrationTest::initialize(); + auto response = sendDownstreamRequest( + [](Http::HeaderMap& headers) { headers.addCopy(LowerCaseString("foo"), "yes"); }); + + // The side stream get the request and sends back the response. + processRequestHeadersMessage(http_side_upstreams_[0], true, absl::nullopt); + + // The request is sent to the upstream. + handleUpstreamRequest(); + EXPECT_THAT(upstream_request_->headers(), SingleHeaderValueIs("foo", "yes")); + + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + verifyDownstreamResponse(*response, 200); +} + +// Side stream server does not mutate the response header. +TEST_P(ExtProcHttpClientIntegrationTest, ServerNoResponseHeaderMutation) { + proto_config_.mutable_processing_mode()->set_request_header_mode(ProcessingMode::SKIP); + + initializeConfig(); + HttpIntegrationTest::initialize(); + auto response = sendDownstreamRequest( + [](Http::HeaderMap& headers) { headers.addCopy(LowerCaseString("foo"), "yes"); }); + + // The request is sent to the upstream. + handleUpstreamRequest(); + EXPECT_THAT(upstream_request_->headers(), SingleHeaderValueIs("foo", "yes")); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + processResponseHeadersMessage(http_side_upstreams_[0], true, absl::nullopt); + verifyDownstreamResponse(*response, 200); +} + +// Side stream server adds and removes headers from the header request. +TEST_P(ExtProcHttpClientIntegrationTest, GetAndSetHeadersWithMutation) { + proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SKIP); + initializeConfig(); + HttpIntegrationTest::initialize(); + auto response = sendDownstreamRequest( + [](Http::HeaderMap& headers) { headers.addCopy(LowerCaseString("x-remove-this"), "yes"); }); + + processRequestHeadersMessage( + http_side_upstreams_[0], true, [](const HttpHeaders& headers, HeadersResponse& headers_resp) { + Http::TestRequestHeaderMapImpl expected_request_headers{ + {":scheme", "http"}, {":method", "GET"}, {"host", "host"}, + {":path", "/"}, {"x-remove-this", "yes"}, {"x-forwarded-proto", "http"}}; + EXPECT_THAT(headers.headers(), HeaderProtosEqual(expected_request_headers)); + + auto response_header_mutation = headers_resp.mutable_response()->mutable_header_mutation(); + auto* mut1 = response_header_mutation->add_set_headers(); + mut1->mutable_header()->set_key("x-new-header"); + mut1->mutable_header()->set_raw_value("new"); + response_header_mutation->add_remove_headers("x-remove-this"); + return true; + }); + + // The request is sent to the upstream. + handleUpstreamRequest(); + EXPECT_THAT(upstream_request_->headers(), SingleHeaderValueIs("x-new-header", "new")); + EXPECT_THAT(upstream_request_->headers(), HasNoHeader("x-remove-this")); + + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + verifyDownstreamResponse(*response, 200); +} + +// Side stream server does not send response trigger timeout. +TEST_P(ExtProcHttpClientIntegrationTest, ServerNoResponseFilterTimeout) { + proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SKIP); + initializeConfig(); + HttpIntegrationTest::initialize(); + auto response = sendDownstreamRequest(absl::nullopt); + + processRequestHeadersMessage(http_side_upstreams_[0], true, + [this](const HttpHeaders&, HeadersResponse&) { + // Travel forward 400 ms exceeding 200ms filter timeout. + timeSystem().advanceTimeWaitImpl(std::chrono::milliseconds(400)); + return false; + }); + // ext_proc filter timeouts sends a 504 local reply depending on runtime flag. + verifyDownstreamResponse(*response, 504); +} + +// Http timeout value set to 10ms. Test HTTP timeout. +TEST_P(ExtProcHttpClientIntegrationTest, ServerResponseHttpClientTimeout) { + ConfigOptions config_option = {}; + config_option.timeout = 10000000; + proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SKIP); + + initializeConfig(config_option); + HttpIntegrationTest::initialize(); + auto response = sendDownstreamRequest(absl::nullopt); + + processRequestHeadersMessage(http_side_upstreams_[0], true, + [this](const HttpHeaders&, HeadersResponse&) { + // Travel forward 50 ms exceeding 10ms HTTP URI timeout setting. + timeSystem().advanceTimeWaitImpl(std::chrono::milliseconds(50)); + return true; + }); + + // HTTP client timeouts sends a 500 local reply. + verifyDownstreamResponse(*response, 500); +} + +// Side stream server sends back 400 with fail-mode-allow set to false. +TEST_P(ExtProcHttpClientIntegrationTest, ServerSendsBackBadRequestFailClose) { + proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SKIP); + initializeConfig(); + HttpIntegrationTest::initialize(); + auto response = sendDownstreamRequest(absl::nullopt); + + processRequestHeadersMessage( + http_side_upstreams_[0], true, [](const HttpHeaders&, HeadersResponse&) { return true; }, + true); + + verifyDownstreamResponse(*response, 500); +} + +// Side stream server sends back 400 with fail-mode-allow set to true. +TEST_P(ExtProcHttpClientIntegrationTest, ServerSendsBackBadRequestFailOpen) { + ConfigOptions config_option = {}; + config_option.failure_mode_allow = true; + proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SKIP); + initializeConfig(config_option); + HttpIntegrationTest::initialize(); + auto response = sendDownstreamRequest(absl::nullopt); + + processRequestHeadersMessage( + http_side_upstreams_[0], true, [](const HttpHeaders&, HeadersResponse&) { return true; }, + true); + + // The request is sent to the upstream. + handleUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + verifyDownstreamResponse(*response, 200); +} + +// Send headers in both directions. +TEST_P(ExtProcHttpClientIntegrationTest, SentHeadersInBothDirection) { + proto_config_.mutable_processing_mode()->set_request_header_mode(ProcessingMode::SEND); + proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SEND); + + initializeConfig(); + HttpIntegrationTest::initialize(); + auto response = sendDownstreamRequestWithBodyAndTrailer("foo"); + + processRequestHeadersMessage( + http_side_upstreams_[0], true, [](const HttpHeaders& headers, HeadersResponse& headers_resp) { + Http::TestRequestHeaderMapImpl expected_request_headers{{":scheme", "http"}, + {":method", "GET"}, + {"host", "host"}, + {":path", "/"}, + {"x-forwarded-proto", "http"}}; + EXPECT_THAT(headers.headers(), HeaderProtosEqual(expected_request_headers)); + + auto response_header_mutation = headers_resp.mutable_response()->mutable_header_mutation(); + auto* mut1 = response_header_mutation->add_set_headers(); + mut1->mutable_header()->set_key("x-new-header"); + mut1->mutable_header()->set_raw_value("new"); + return true; + }); + + // The request is sent to the upstream. + handleUpstreamRequestWithTrailer(); + EXPECT_THAT(upstream_request_->headers(), SingleHeaderValueIs("x-new-header", "new")); + EXPECT_EQ(upstream_request_->body().toString(), "foo"); + + processResponseHeadersMessage(http_side_upstreams_[0], false, absl::nullopt); + verifyDownstreamResponse(*response, 200); +} + +// Wrong ext_proc filter cluster config with fail close. +TEST_P(ExtProcHttpClientIntegrationTest, WrongClusterConfigWithFailClose) { + ConfigOptions config_option = {}; + config_option.failure_mode_allow = false; + config_option.cluster = "foo"; + proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SKIP); + + initializeConfig(config_option); + HttpIntegrationTest::initialize(); + auto response = sendDownstreamRequest(absl::nullopt); + verifyDownstreamResponse(*response, 504); +} + +// Wrong ext_proc filter cluster config with fail open +TEST_P(ExtProcHttpClientIntegrationTest, WrongClusterConfigWithFailOpen) { + ConfigOptions config_option = {}; + config_option.failure_mode_allow = true; + config_option.cluster = "foo"; + proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SKIP); + + initializeConfig(config_option); + HttpIntegrationTest::initialize(); + auto response = sendDownstreamRequest(absl::nullopt); + // The request is sent to the upstream. + handleUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + verifyDownstreamResponse(*response, 200); +} + +} // namespace +} // namespace ExternalProcessing +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/ext_proc/http_client/http_client_test.cc b/test/extensions/filters/http/ext_proc/http_client/http_client_test.cc index 27a6d83c9e..faca0383df 100644 --- a/test/extensions/filters/http/ext_proc/http_client/http_client_test.cc +++ b/test/extensions/filters/http/ext_proc/http_client/http_client_test.cc @@ -18,10 +18,26 @@ class ExtProcHttpClientTest : public testing::Test { public: ~ExtProcHttpClientTest() override = default; - void SetUp() override { client_ = std::make_unique(config_, context_); } + const std::string default_http_config_ = R"EOF( + http_service: + http_service: + http_uri: + uri: "ext_proc_server_0:9000" + cluster: "ext_proc_server_0" + timeout: + seconds: 500 + processing_mode: + request_header_mode: "SEND" + response_header_mode: "SKIP" + )EOF"; + + void SetUp() override { + TestUtility::loadFromYaml(default_http_config_, config_); + client_ = std::make_unique(config_, context_); + } protected: - envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor config_; + envoy::extensions::filters::http::ext_proc::v3::ExternalProcessor config_{}; testing::NiceMock context_; Upstream::MockClusterManager& cm_{context_.cluster_manager_}; std::unique_ptr client_; @@ -31,30 +47,49 @@ class ExtProcHttpClientTest : public testing::Test { TEST_F(ExtProcHttpClientTest, Basic) { SetUp(); - client_->sendRequest(); + client_->cancel(); client_->context(); Tracing::MockSpan parent_span; client_->onBeforeFinalizeUpstreamSpan(parent_span, nullptr); Http::AsyncClient::FailureReason reason = Envoy::Http::AsyncClient::FailureReason::Reset; client_->onFailure(async_request_, reason); + reason = Envoy::Http::AsyncClient::FailureReason::ExceedResponseBufferLimit; + client_->onFailure(async_request_, reason); +} - Http::ResponseHeaderMapPtr resp_headers_ok(new Http::TestResponseHeaderMapImpl({ +TEST_F(ExtProcHttpClientTest, JsonDecodingErrorTest) { + Http::ResponseHeaderMapPtr resp_headers(new Http::TestResponseHeaderMapImpl({ {":status", "200"}, })); - Http::ResponseMessagePtr response_ok(new Http::ResponseMessageImpl(std::move(resp_headers_ok))); - client_->onSuccess(async_request_, std::move(response_ok)); + Http::ResponseMessagePtr response_ok_with_bad_body( + new Http::ResponseMessageImpl(std::move(resp_headers))); + response_ok_with_bad_body->body().add("foo-bar"); + client_->onSuccess(async_request_, std::move(response_ok_with_bad_body)); +} +TEST_F(ExtProcHttpClientTest, EmptyResponseBodyTest) { + Http::ResponseHeaderMapPtr resp_headers(new Http::TestResponseHeaderMapImpl({ + {":status", "200"}, + })); + Http::ResponseMessagePtr response_ok_with_empty_body( + new Http::ResponseMessageImpl(std::move(resp_headers))); + client_->onSuccess(async_request_, std::move(response_ok_with_empty_body)); +} + +TEST_F(ExtProcHttpClientTest, ResponseStatusNotOkTest) { Http::ResponseHeaderMapPtr resp_headers(new Http::TestResponseHeaderMapImpl({ {":status", "403"}, })); Http::ResponseMessagePtr response(new Http::ResponseMessageImpl(std::move(resp_headers))); client_->onSuccess(async_request_, std::move(response)); +} - Http::ResponseHeaderMapPtr resp_headers_foo(new Http::TestResponseHeaderMapImpl({ +TEST_F(ExtProcHttpClientTest, WrongResponseStatusTest) { + Http::ResponseHeaderMapPtr resp_headers(new Http::TestResponseHeaderMapImpl({ {":status", "foo"}, })); - Http::ResponseMessagePtr response_foo(new Http::ResponseMessageImpl(std::move(resp_headers_foo))); + Http::ResponseMessagePtr response_foo(new Http::ResponseMessageImpl(std::move(resp_headers))); client_->onSuccess(async_request_, std::move(response_foo)); } diff --git a/test/extensions/filters/http/ext_proc/mock_server.cc b/test/extensions/filters/http/ext_proc/mock_server.cc index 29637f793f..35c91b238e 100644 --- a/test/extensions/filters/http/ext_proc/mock_server.cc +++ b/test/extensions/filters/http/ext_proc/mock_server.cc @@ -5,12 +5,17 @@ namespace Extensions { namespace HttpFilters { namespace ExternalProcessing { -MockClient::MockClient() { - EXPECT_CALL(*this, stream()).WillRepeatedly(testing::Invoke([this]() { return stream_; })); +using ::testing::_; +using ::testing::Invoke; - EXPECT_CALL(*this, setStream(testing::_)) +MockClient::MockClient() { + EXPECT_CALL(*this, sendRequest(_, _, _, _, _)) .WillRepeatedly( - testing::Invoke([this](ExternalProcessorStream* stream) -> void { stream_ = stream; })); + Invoke([](envoy::service::ext_proc::v3::ProcessingRequest&& request, bool end_stream, + const uint64_t, RequestCallbacks*, StreamBase* stream) { + ExternalProcessorStream* grpc_stream = dynamic_cast(stream); + grpc_stream->send(std::move(request), end_stream); + })); } MockClient::~MockClient() = default; diff --git a/test/extensions/filters/http/ext_proc/mock_server.h b/test/extensions/filters/http/ext_proc/mock_server.h index 12c9d7308a..a4962acb4e 100644 --- a/test/extensions/filters/http/ext_proc/mock_server.h +++ b/test/extensions/filters/http/ext_proc/mock_server.h @@ -13,14 +13,15 @@ class MockClient : public ExternalProcessorClient { public: MockClient(); ~MockClient() override; + MOCK_METHOD(ExternalProcessorStreamPtr, start, (ExternalProcessorCallbacks&, const Grpc::GrpcServiceConfigWithHashKey&, const Envoy::Http::AsyncClient::StreamOptions&, Envoy::Http::StreamFilterSidestreamWatermarkCallbacks&)); - MOCK_METHOD(ExternalProcessorStream*, stream, ()); - MOCK_METHOD(void, setStream, (ExternalProcessorStream * stream)); - - ExternalProcessorStream* stream_ = nullptr; + MOCK_METHOD(void, sendRequest, + (envoy::service::ext_proc::v3::ProcessingRequest&&, bool, const uint64_t, + RequestCallbacks*, StreamBase*)); + MOCK_METHOD(void, cancel, ()); }; class MockStream : public ExternalProcessorStream { diff --git a/test/extensions/filters/http/ext_proc/ordering_test.cc b/test/extensions/filters/http/ext_proc/ordering_test.cc index 4b1e4cf133..a12be075b0 100644 --- a/test/extensions/filters/http/ext_proc/ordering_test.cc +++ b/test/extensions/filters/http/ext_proc/ordering_test.cc @@ -76,7 +76,7 @@ class OrderingTest : public testing::Test { std::make_shared( Envoy::Extensions::Filters::Common::Expr::createBuilder(nullptr)), factory_context_); - filter_ = std::make_unique(config_, std::move(client_), proto_config.grpc_service()); + filter_ = std::make_unique(config_, std::move(client_)); filter_->setEncoderFilterCallbacks(encoder_callbacks_); filter_->setDecoderFilterCallbacks(decoder_callbacks_); } diff --git a/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_unit_test_fuzz.cc b/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_unit_test_fuzz.cc index 25c6b3ac2d..f8635e1859 100644 --- a/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_unit_test_fuzz.cc +++ b/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_unit_test_fuzz.cc @@ -98,7 +98,7 @@ DEFINE_PROTO_FUZZER( ExternalProcessing::MockClient* client = new ExternalProcessing::MockClient(); std::unique_ptr filter = std::make_unique( - config, ExternalProcessing::ExternalProcessorClientPtr{client}, proto_config.grpc_service()); + config, ExternalProcessing::ExternalProcessorClientPtr{client}); filter->setDecoderFilterCallbacks(mocks.decoder_callbacks_); filter->setEncoderFilterCallbacks(mocks.encoder_callbacks_); diff --git a/test/extensions/filters/http/ext_proc/utils.h b/test/extensions/filters/http/ext_proc/utils.h index 858e8e7cb0..351d40bd74 100644 --- a/test/extensions/filters/http/ext_proc/utils.h +++ b/test/extensions/filters/http/ext_proc/utils.h @@ -31,6 +31,10 @@ MATCHER_P(HasNoHeader, key, absl::StrFormat("Headers have no value for \"%s\"", return arg.get(::Envoy::Http::LowerCaseString(std::string(key))).empty(); } +MATCHER_P(HasHeader, key, absl::StrFormat("There exists a header for \"%s\"", key)) { + return !arg.get(::Envoy::Http::LowerCaseString(std::string(key))).empty(); +} + MATCHER_P2(SingleHeaderValueIs, key, value, absl::StrFormat("Header \"%s\" equals \"%s\"", key, value)) { const auto hdr = arg.get(::Envoy::Http::LowerCaseString(std::string(key))); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 921e5ba809..510cd6924d 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -4975,10 +4975,10 @@ TEST_P(ProtocolIntegrationTest, InvalidResponseHeaderNameStreamError) { EXPECT_EQ("502", response->headers().getStatusValue()); test_server_->waitForCounterGe("http.config_test.downstream_rq_5xx", 1); - std::string error_message = - upstreamProtocol() == Http::CodecType::HTTP3 - ? "upstream_reset_before_response_started{protocol_error|QUIC_BAD_APPLICATION_PAYLOAD}" - : "upstream_reset_before_response_started{protocol_error}"; + std::string error_message = upstreamProtocol() == Http::CodecType::HTTP3 + ? "upstream_reset_before_response_started{protocol_error|QUIC_" + "BAD_APPLICATION_PAYLOAD|FROM_SELF}" + : "upstream_reset_before_response_started{protocol_error}"; EXPECT_EQ(waitForAccessLog(access_log_name_), error_message); // Upstream connection should stay up diff --git a/test/integration/quic_http_integration_test.cc b/test/integration/quic_http_integration_test.cc index 8be734d474..f859f5d2b0 100644 --- a/test/integration/quic_http_integration_test.cc +++ b/test/integration/quic_http_integration_test.cc @@ -241,7 +241,7 @@ class QuicHttpIntegrationTestBase : public HttpIntegrationTest { quic::QuicServerId{ (host.empty() ? transport_socket_factory_->clientContextConfig()->serverNameIndication() : host), - static_cast(port), false}, + static_cast(port)}, transport_socket_factory_->getCryptoConfig(), *dispatcher_, // Use smaller window than the default one to have test coverage of client codec buffer // exceeding high watermark. @@ -1089,7 +1089,8 @@ TEST_P(QuicHttpIntegrationTest, CertVerificationFailure) { std::string failure_reason = "QUIC_TLS_CERTIFICATE_UNKNOWN with details: TLS handshake failure " "(ENCRYPTION_HANDSHAKE) 46: " "certificate unknown. SSLErrorStack:"; - EXPECT_EQ(failure_reason, codec_client_->connection()->transportFailureReason()); + EXPECT_THAT(codec_client_->connection()->transportFailureReason(), + testing::HasSubstr(failure_reason)); } TEST_P(QuicHttpIntegrationTest, ResetRequestWithoutAuthorityHeader) { diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h index f153a22aa2..2d23f31794 100644 --- a/test/mocks/config/mocks.h +++ b/test/mocks/config/mocks.h @@ -131,6 +131,13 @@ class MockGrpcMux : public GrpcMux { MOCK_METHOD(bool, paused, (const std::string& type_url), (const)); MOCK_METHOD(EdsResourcesCacheOptRef, edsResourcesCache, ()); + + MOCK_METHOD(absl::Status, updateMuxSource, + (Grpc::RawAsyncClientPtr && primary_async_client, + Grpc::RawAsyncClientPtr&& failover_async_client, + CustomConfigValidatorsPtr&& custom_config_validators, Stats::Scope& scope, + BackOffStrategyPtr&& backoff_strategy, + const envoy::config::core::v3::ApiConfigSource& ads_config_source)); }; class MockGrpcStreamCallbacks