securitycenter: make streaming_config.filter optional on NotificationConfig (#11021)#75
Open
jbbqqf wants to merge 11 commits into
Open
securitycenter: make streaming_config.filter optional on NotificationConfig (#11021)#75jbbqqf wants to merge 11 commits into
jbbqqf wants to merge 11 commits into
Conversation
…Config (GoogleCloudPlatform#11021) The SCC NotificationConfig API marshals StreamingConfig.Filter with json:"filter,omitempty", indicating the field is optional server-side. However the mmv1 schema marks streamingConfig.filter as required, which forces users to set a non-empty filter even when they want all events. This change drops required: true from filter for all six NotificationConfig variants (org / folder / project, both v1 and v2). The outer streamingConfig remains required because the resource still needs the streaming config block to know what kind of notifications to emit. Fixes hashicorp/terraform-provider-google#11021 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Drop
required: truefromstreamingConfig.filteron every variant ofgoogle_scc_notification_config(organization / folder / project, bothsecuritycenterv1 andsecuritycenterv2). The SCC API marks the fieldas optional in the wire format, and users have asked to omit it for "all
events" notifications.
Fixes hashicorp/terraform-provider-google#11021 — see hashicorp/terraform-provider-google#11021
Why
The Security Command Center notification API serializes
StreamingConfig.Filterwithomitempty(see the vendored Go APIclient
securitycenter/v1/securitycenter-gen.go):omitemptyon a non-pointer string means "skip the field on the wire ifunset", which the API documentation confirms by listing filter as
optional and saying "a list of zero or more restrictions". Yet the mmv1
schema marks
streamingConfig.filterasrequired: truefor all sixvariants:
mmv1/products/securitycenter/NotificationConfig.yamlmmv1/products/securitycenter/FolderNotificationConfig.yamlmmv1/products/securitycenter/ProjectNotificationConfig.yamlmmv1/products/securitycenterv2/FolderNotificationConfig.yamlmmv1/products/securitycenterv2/OrganizationNotificationConfig.yamlmmv1/products/securitycenterv2/ProjectNotificationConfig.yamlSo a user who wants notifications for every create/update event
(the natural "I just want everything" case) must invent a filter that
matches everything — there is no "always true" expression in the SCC
filter grammar and the trivial workarounds are awkward.
GCP API reference:
What changed
Pure mmv1 schema change: removed the inner
required: truefrom thefilterproperty in all six*NotificationConfig.yamlfiles. Theouter
streamingConfigblock is stillrequired: truebecause everyNotificationConfig must specify a streaming config (that's the only
config kind the resource supports today).
After regen, the generated TPG schema for
streaming_config.filterbecomes
Optional: trueinstead ofRequired: true. No expand orflatten code needed to change.
Edge cases tested
streaming_config { }Error: Missing required argument: filter.Optional: truein generated schema) +make buildcleanfilter = "category = \"OPEN_FIREWALL\" AND state = \"ACTIVE\""scc_notification_config_basicfilter = ""after a previous non-empty valuestreamingConfig.filterupdate mask is preserved (update_mask_fields: streamingConfig.filteris unchanged). API accepts empty filter on PATCH.update_mask_fieldsblock untouchedTest protocol
yq eval .) on all 6 filesmake build OUTPUT_PATH=... VERSION=gago build ./google/services/securitycenter/...on regenerated TPGgo build ./...on full regenerated TPGscc_notification_config_basicacceptance test covers the with-filter case. The without-filter case requires an SCC organization or folder, which the author does not have access to in the sandbox.I'd appreciate maintainer verification on a sandbox SCC org if a live
acceptance run is desired before merge.
Resources
google.golang.org/api@v0.278.0/securitycenter/v1/securitycenter-gen.go(StreamingConfig.Filter hasjson:"filter,omitempty")Disclosure
This PR was drafted with assistance from Claude Code as part of a focused
contribution batch on TPG schema bugs. The diff was reviewed manually
against the GCP REST API documentation and the vendored Go client. The
mmv1 generation was run locally and the regenerated provider compiled
clean; a live BEFORE/AFTER smoke run was not done because the resource
needs a real SCC organization, which the author does not have set up.
The author (a human) reviewed the diff and the regenerated schema before
opening this PR.