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

Control plane support for Extractor regex replace #9124

Merged
merged 82 commits into from
Mar 14, 2024

Conversation

ben-taussig-solo
Copy link
Contributor

@ben-taussig-solo ben-taussig-solo commented Jan 31, 2024

Description

User-facing API

  • Extractors now support a mode field, which represents the type of extraction to perform. The mode field can be one of EXTRACT, SINGLE_REPLACE, or REPLACE_ALL. The mode field is optional, and defaults to EXTRACT (prior to these changes, EXTRACT was the standard behavior for extractors).
  // The mode of operation for the extraction.
  enum Mode {
    // Default mode. Extract the value of the subgroup-th capturing group.
    EXTRACT = 0; 
    // Replace the value of the subgroup-th capturing group with the replacement_text.
    // Note: replacement_text must be set for this mode.
    SINGLE_REPLACE = 1;
    // Replace all matches of the regex in the source with the replacement_text.
    // Note: replacement_text must be set for this mode.
    // Note: subgroup is ignored for this mode. configuration will fail if subgroup is set.
    // Note: restrictions on the regex are different for this mode. See the regex field for more details.
    REPLACE_ALL = 2;
  }
  • Extractors now have a replacement_text field, which is used in SINGLE_REPLACE and REPLACE_ALL modes. This field is optional, and is required if the extractor is configured in SINGLE_REPLACE or REPLACE_ALL mode.
  // The string to replace the matched portion of the source with.
  // Used in SINGLE_REPLACE and REPLACE_ALL modes.
  google.protobuf.StringValue replacement_text = 5;

Control-plane Updates

  • The control-plane now handles the new mode extractor field,
  • The control-plane now has configuration validation behavior unique to each mode
    • If the mode is EXTRACT, the control-plane will error if replacement_text is set.
    • If the mode is SINGLE_REPLACE, the control-plane will error if replacement_text is not set.
    • If the mode is REPLACE_ALL, the control-plane will error if subgroup is set, and if replacement_text is not set.

Testing

  • This PR adds unit tests for the new functionality, ensuring that the control-plane correctly handles the new mode and replacement_text fields, and that invalid configurations are rejected.
  • This PR also adds e2e tests for the new functionality, ensuring that the new functionality interacts with the data plane as expected.

Additional Bug Fixes

  • Additionally, this PR adds control-plane validation of regex/subgroup configuration, mirroring validation done in the envoy-gloo transformation filter
    • The check in the transformation filter evaluates whether the specified subgroup is larger than the total number of capturing groups in the regex pattern. If so, the filter throws an exception, preventing envoy from handling any traffic altogether until the configuration is fixed.
    • This PR duplicates that check in the control-plane, preventing this misconfiguration from being pushed to the envoy proxy in the first place.
    • However, the check requires the regex pattern to be compiled, which will hurt translation performance, especially if many regexes are being validated. If you have qualms about this, I am happy to move these changes into a separate PR. where we can discuss the tradeoffs in more detail.

Remaining Work

  • I need to make substantial changes to the documentation before this work is complete. I may choose to handle that in a separate PR if that seems more straightforward
    • Update: these changes have been added
  • I am going to leave this PR in WIP/draft state until the corresponding envoy-gloo PR is merged and released: Add regex replace functionality to transformation filter extractors [Revised] envoy-gloo#309
    • Update: The envoy-gloo PR has been merged and released and I have marked this PR ready for review

@github-actions github-actions bot added the keep pr updated signals bulldozer to keep pr up to date with base branch label Jan 31, 2024
Makefile Outdated Show resolved Hide resolved
jbohanon
jbohanon previously approved these changes Mar 13, 2024
@ben-taussig-solo ben-taussig-solo enabled auto-merge (squash) March 13, 2024 18:36
@ben-taussig-solo ben-taussig-solo merged commit 128d243 into main Mar 14, 2024
20 checks passed
@ben-taussig-solo ben-taussig-solo deleted the extractor_regex_replace branch March 14, 2024 13:20
ben-taussig-solo added a commit that referenced this pull request Mar 14, 2024
* update envoy transformation API w changes as of 55ddd1bfc4d9976bc953cab2dd0fcc874edd9405

* add new settings to user-facing API

* update transformation plugin + add unit tests

* bump envoy-gloo version to pre-release hash

* unit testing updates

* check regex validity in transformation plugin

* use table based tests

* add e2e tests

* update comment, add DO_NOT_SUBMIT tag

* testing updates

* create error type

* support dynamic_metadata API field

* support dynamic_metadata API field in user-facing API

* Revert "support dynamic_metadata API field in user-facing API"

This reverts commit 1602aba.

* Revert "support dynamic_metadata API field"

This reverts commit a001c66.

* update inline documentation of transformation filter to match envoy-gloo changes

* add changelog entry

* use pre-release ENVOY_GLOO_IMAGE

* remove DO_NOT_MERGE, add new test against default mode

* add documentation for new feature

* clean up plugin.go todos and commentary

* update external API documentation

* remove remaining TODO in plugin.test.go

* unfocus tests in plugin_test.go

* let control plane be dumb about regex semantics; already covered by envoy validation mode

* update proto documentation for new modes

* docs updates

* Adding changelog file to new location

* Deleting changelog file from old location

* bump ENVOY_GLOO_IMAGE to 1.28.1-patch2

* documentation updates

* update user guide, .proto inline documentation

* remove unintentially committed bananaToOrange test

* resolve typos in .proto files

* update tests with new defaults, move envoy transformation validation tests to kube2e suite

* Adding changelog file to new location

* Deleting changelog file from old location

* update envoy-gloo dep to point to new 1.27 release

* empty commit to kick build

* clean up modeName check when generating extractor errors

Co-authored-by: Jacob Bohanon <jacob.bohanon@solo.io>

* remove duplicated default extraction mode handler

* validate new default mode handling in new test

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: changelog-bot <changelog-bot>
Co-authored-by: Jacob Bohanon <jacob.bohanon@solo.io>
ben-taussig-solo added a commit that referenced this pull request Mar 14, 2024
* update envoy transformation API w changes as of 55ddd1bfc4d9976bc953cab2dd0fcc874edd9405

* add new settings to user-facing API

* update transformation plugin + add unit tests

* bump envoy-gloo version to pre-release hash

* unit testing updates

* check regex validity in transformation plugin

* use table based tests

* add e2e tests

* update comment, add DO_NOT_SUBMIT tag

* testing updates

* create error type

* support dynamic_metadata API field

* support dynamic_metadata API field in user-facing API

* Revert "support dynamic_metadata API field in user-facing API"

This reverts commit 1602aba.

* Revert "support dynamic_metadata API field"

This reverts commit a001c66.

* update inline documentation of transformation filter to match envoy-gloo changes

* add changelog entry

* use pre-release ENVOY_GLOO_IMAGE

* remove DO_NOT_MERGE, add new test against default mode

* add documentation for new feature

* clean up plugin.go todos and commentary

* update external API documentation

* remove remaining TODO in plugin.test.go

* unfocus tests in plugin_test.go

* let control plane be dumb about regex semantics; already covered by envoy validation mode

* update proto documentation for new modes

* docs updates

* Adding changelog file to new location

* Deleting changelog file from old location

* bump ENVOY_GLOO_IMAGE to 1.28.1-patch2

* documentation updates

* update user guide, .proto inline documentation

* remove unintentially committed bananaToOrange test

* resolve typos in .proto files

* update tests with new defaults, move envoy transformation validation tests to kube2e suite

* Adding changelog file to new location

* Deleting changelog file from old location

* update envoy-gloo dep to point to new 1.27 release

* empty commit to kick build

* clean up modeName check when generating extractor errors

Co-authored-by: Jacob Bohanon <jacob.bohanon@solo.io>

* remove duplicated default extraction mode handler

* validate new default mode handling in new test

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: changelog-bot <changelog-bot>
Co-authored-by: Jacob Bohanon <jacob.bohanon@solo.io>
soloio-bulldozer bot added a commit that referenced this pull request Mar 14, 2024
* Control plane support for Extractor regex replace (#9124)

* update envoy transformation API w changes as of 55ddd1bfc4d9976bc953cab2dd0fcc874edd9405

* add new settings to user-facing API

* update transformation plugin + add unit tests

* bump envoy-gloo version to pre-release hash

* unit testing updates

* check regex validity in transformation plugin

* use table based tests

* add e2e tests

* update comment, add DO_NOT_SUBMIT tag

* testing updates

* create error type

* support dynamic_metadata API field

* support dynamic_metadata API field in user-facing API

* Revert "support dynamic_metadata API field in user-facing API"

This reverts commit 1602aba.

* Revert "support dynamic_metadata API field"

This reverts commit a001c66.

* update inline documentation of transformation filter to match envoy-gloo changes

* add changelog entry

* use pre-release ENVOY_GLOO_IMAGE

* remove DO_NOT_MERGE, add new test against default mode

* add documentation for new feature

* clean up plugin.go todos and commentary

* update external API documentation

* remove remaining TODO in plugin.test.go

* unfocus tests in plugin_test.go

* let control plane be dumb about regex semantics; already covered by envoy validation mode

* update proto documentation for new modes

* docs updates

* Adding changelog file to new location

* Deleting changelog file from old location

* bump ENVOY_GLOO_IMAGE to 1.28.1-patch2

* documentation updates

* update user guide, .proto inline documentation

* remove unintentially committed bananaToOrange test

* resolve typos in .proto files

* update tests with new defaults, move envoy transformation validation tests to kube2e suite

* Adding changelog file to new location

* Deleting changelog file from old location

* update envoy-gloo dep to point to new 1.27 release

* empty commit to kick build

* clean up modeName check when generating extractor errors

Co-authored-by: Jacob Bohanon <jacob.bohanon@solo.io>

* remove duplicated default extraction mode handler

* validate new default mode handling in new test

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: changelog-bot <changelog-bot>
Co-authored-by: Jacob Bohanon <jacob.bohanon@solo.io>

* update changelog location

* respsect updated variable name in kube2e gateway_test.go

* update language in user guide

* correct typo in .proto documentation

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Jacob Bohanon <jacob.bohanon@solo.io>
soloio-bulldozer bot added a commit that referenced this pull request Mar 15, 2024
* Control plane support for Extractor regex replace (#9124)

* update envoy transformation API w changes as of 55ddd1bfc4d9976bc953cab2dd0fcc874edd9405

* add new settings to user-facing API

* update transformation plugin + add unit tests

* bump envoy-gloo version to pre-release hash

* unit testing updates

* check regex validity in transformation plugin

* use table based tests

* add e2e tests

* update comment, add DO_NOT_SUBMIT tag

* testing updates

* create error type

* support dynamic_metadata API field

* support dynamic_metadata API field in user-facing API

* Revert "support dynamic_metadata API field in user-facing API"

This reverts commit 1602aba.

* Revert "support dynamic_metadata API field"

This reverts commit a001c66.

* update inline documentation of transformation filter to match envoy-gloo changes

* add changelog entry

* use pre-release ENVOY_GLOO_IMAGE

* remove DO_NOT_MERGE, add new test against default mode

* add documentation for new feature

* clean up plugin.go todos and commentary

* update external API documentation

* remove remaining TODO in plugin.test.go

* unfocus tests in plugin_test.go

* let control plane be dumb about regex semantics; already covered by envoy validation mode

* update proto documentation for new modes

* docs updates

* Adding changelog file to new location

* Deleting changelog file from old location

* bump ENVOY_GLOO_IMAGE to 1.28.1-patch2

* documentation updates

* update user guide, .proto inline documentation

* remove unintentially committed bananaToOrange test

* resolve typos in .proto files

* update tests with new defaults, move envoy transformation validation tests to kube2e suite

* Adding changelog file to new location

* Deleting changelog file from old location

* update envoy-gloo dep to point to new 1.27 release

* empty commit to kick build

* clean up modeName check when generating extractor errors

Co-authored-by: Jacob Bohanon <jacob.bohanon@solo.io>

* remove duplicated default extraction mode handler

* validate new default mode handling in new test

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: changelog-bot <changelog-bot>
Co-authored-by: Jacob Bohanon <jacob.bohanon@solo.io>

* update changelog location

* update changelog entry location

* respsect updated variable name in kube2e gateway_test.go

* update language in user guide

* correct typo in .proto documentation

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Jacob Bohanon <jacob.bohanon@solo.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep pr updated signals bulldozer to keep pr up to date with base branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants