-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: Unexclude remaining configs from validation #13534
docs: Unexclude remaining configs from validation #13534
Conversation
/retest |
Retrying Azure Pipelines, to retry CircleCI checks, use |
/retest |
Retrying Azure Pipelines, to retry CircleCI checks, use |
/retest |
Retrying Azure Pipelines, to retry CircleCI checks, use |
b829a1c
to
f7f90aa
Compare
docs/BUILD
Outdated
filegroup( | ||
name = "configs", | ||
srcs = glob( | ||
["root/**/*.yaml"], | ||
exclude = [ | ||
# `some.customer.filter` | ||
"root/intro/_include/life-of-a-request.yaml", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess its not trivial to add some.customer.filter
- wondering how we can keep this config valid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just use a real filter? @htuch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any suggestions on which filter to use would be great
i think we are going to need to look at the docs related to these configs to see what effect these changes have
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have commented out the some.customer.filter
example from the filter chain - as i think it still serves its purpose being present but commented out - open to suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer it to be included, since it's designed to capture what happens when you add a customer filter and is referenced extensively in the text (as "CustomerFilter"). I wonder if we can somehow augment envoy/tools/type_whisperer/all_protos_with_ext_pb_text.pb_text
to reflect some dummy config.
docs/BUILD
Outdated
"root/intro/arch_overview/security/_include/ssl.yaml", | ||
"root/configuration/http/http_filters/_include/dns-cache-circuit-breaker.yaml", | ||
"root/configuration/http/http_filters/_include/grpc-reverse-bridge-filter.yaml", | ||
# missing proto.pb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im wondering if/how we can include this proto.pb
file into the test bundle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep I guess this need to be part of artifact, or bazelify whole of example test (which is out of scope of this PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be fixed. i generated a .pb
file and added to bazel build etc
i have also updated the docs, as the required steps were not very clear, and the examples were a bit inconsistent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this. This is so great. A few comments.
/wait
docs/BUILD
Outdated
filegroup( | ||
name = "configs", | ||
srcs = glob( | ||
["root/**/*.yaml"], | ||
exclude = [ | ||
# `some.customer.filter` | ||
"root/intro/_include/life-of-a-request.yaml", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just use a real filter? @htuch?
docs/BUILD
Outdated
"root/intro/arch_overview/security/_include/ssl.yaml", | ||
"root/configuration/http/http_filters/_include/dns-cache-circuit-breaker.yaml", | ||
"root/configuration/http/http_filters/_include/grpc-reverse-bridge-filter.yaml", | ||
# missing proto.pb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
138824b
to
78ec197
Compare
to accomodate changes to the example configs, i made some changes to the also the https://storage.googleapis.com/envoy-pr/13534/docs/intro/arch_overview/security/ssl.html |
and the life of a request page https://storage.googleapis.com/envoy-pr/13534/docs/intro/life_of_a_request.html |
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
4ec9292
to
ce06ed6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing. Thank you! LMK if you want to merge this and iterate or keep trying to fix stuff.
ill add the todo - but then should be ready to land i think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
/retest |
Retrying Azure Pipelines, to retry CircleCI checks, use |
exclude = [ | ||
"root/intro/_include/life-of-a-request.yaml", | ||
# TODO(phlax/windows-dev): figure out how to get this working on windows | ||
# "Error: unable to read file: /etc/ssl/certs/ca-certificates.crt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll open an issue for this, it could dovetail with the idea of Envoy tapping into the trusted system cert pool as well potentially?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, will do
Merge main once #13598 merges. Thanks! /wait |
/retest |
Retrying Azure Pipelines, to retry CircleCI checks, use |
* master: (22 commits) delay health checks until transport socket secrets are ready. (envoyproxy#13516) test, oauth2: Make sure config test runs field validation (envoyproxy#13496) [http] swap codec implementations to default new (envoyproxy#13579) wasm: update proxy-wasm-cpp-host (envoyproxy#13606) postgres: do not copy and linearize received data when it is not going to be used (envoyproxy#13393) configs: Update configs v2 -> v3 (envoyproxy#13562) http2: Remove RELEASE_ASSERTs in sendPendingFrames() error handling (envoyproxy#13546) dependencies: track untracked implied dependencies, wrapup dashboard. (envoyproxy#13571) listener: add match all filter chain (envoyproxy#13449) fix mistakes in docstrings (envoyproxy#13603) ratelimit: add route entry metadata to ratelimit actions (envoyproxy#13269) cluster manager: avoid immediate activation for dynamic inserted cluster when initialize (envoyproxy#12783) ext_authz: Avoid calling check multiple times (envoyproxy#13288) docs: Unexclude remaining configs from validation (envoyproxy#13534) build: update rules_rust to allow Rustc in RBE (envoyproxy#13595) docs: Update sphinxext.rediraffe (envoyproxy#13589) Deprecate moonjit support on Windows before beta (envoyproxy#13541) dependencies: bump LuaJIT to 2.1 branch HEAD @ e9af1ab. (envoyproxy#13474) docs: add TLS stats to cluster stats doc (envoyproxy#13561) ci: stop building alpine-debug images in favor of ubuntu-based debug image (envoyproxy#13598) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Signed-off-by: Ryan Northey ryan@synca.io
Commit Message: docs: Unexclude remaining configs from validation
Additional Description:
some configs in docs were initially excluded from validation (in #13387) - this PR will remove the exclusions where possible
Risk Level:
Testing:
Docs Changes:
Release Notes:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]