Skip to content
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

[xDS] pass HTTP filter name to GenerateServiceConfig() method. #32976

Merged
merged 3 commits into from May 1, 2023

Conversation

rockspore
Copy link
Contributor

We need the RBAC filter name as the policy_name field in audit logging context.

@rockspore rockspore requested a review from gtcooke94 May 1, 2023 17:47
@rockspore rockspore requested a review from markdroth as a code owner May 1, 2023 17:47
@rockspore rockspore added the release notes: no Indicates if PR should not be in release notes label May 1, 2023
@markdroth markdroth changed the title [Audit Logging] pass filter name to generate server config [xDS] pass HTTP filter name to GenerateServiceConfig() method May 1, 2023
Copy link
Member

@markdroth markdroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Feel free to merge after addressing the outstanding comment.

@@ -303,7 +303,8 @@ TEST_F(XdsFaultInjectionFilterTest, ModifyChannelArgs) {
TEST_F(XdsFaultInjectionFilterTest, GenerateServiceConfigTopLevelConfig) {
XdsHttpFilterImpl::FilterConfig config;
config.config = Json::Object{{"foo", "bar"}};
auto service_config = filter_->GenerateServiceConfig(config, nullptr);
auto service_config =
filter_->GenerateServiceConfig(config, nullptr, "" /*filter_name=*/);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this to be /*filter_name=*/"".

Same thing throughout.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Mark! Fixed.

@rockspore rockspore changed the title [xDS] pass HTTP filter name to GenerateServiceConfig() method [xDS] pass HTTP filter name to GenerateServiceConfig() method. May 1, 2023
Copy link
Contributor

@gtcooke94 gtcooke94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with 1 comment

src/core/ext/xds/xds_http_rbac_filter.cc Show resolved Hide resolved
@rockspore rockspore enabled auto-merge (squash) May 1, 2023 22:05
@rockspore rockspore merged commit f02ce24 into grpc:master May 1, 2023
63 of 64 checks passed
@rockspore rockspore deleted the filter-name branch May 1, 2023 22:10
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label May 2, 2023
paulosjca pushed a commit to paulosjca/grpc that referenced this pull request May 4, 2023
…c#32976)

We need the RBAC filter name as the `policy_name` field in audit logging
context.
wanlin31 pushed a commit that referenced this pull request May 18, 2023
)

We need the RBAC filter name as the `policy_name` field in audit logging
context.
@yijiem yijiem added release notes: yes Indicates if PR needs to be in release notes and removed release notes: no Indicates if PR should not be in release notes labels May 31, 2023
@erm-g erm-g removed the release notes: yes Indicates if PR needs to be in release notes label Jun 12, 2023
@yijiem yijiem added the release notes: no Indicates if PR should not be in release notes label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bloat/low imported Specifies if the PR has been imported to the internal repository lang/core per-call-memory/neutral per-channel-memory/neutral release notes: no Indicates if PR should not be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants